Skip to content

[SYCL] Fix the check for read-only host pointer during memobj creation #2697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 3, 2020

Conversation

sergey-semenov
Copy link
Contributor

Previously, whenever a native memory object was created, the original host
pointer passed by the user was checked for being read-only, regardless
of what host pointer was actually used at that point. This patch fixes the
issue by assuming a writeable host pointer unless it's the one provided by
the user.

As a result, this fixes a double free issue that appeared with the
following usage pattern:

  1. Create a buffer with a const user pointer
  2. Write-access it on host
  3. Access it on a non-host device
  4. Access it on host

This error was caused by the allocation performed at step 2 being
treated as read-only and, therefore, used with
PI_MEM_FLAGS_HOST_PTR_COPY rather than PI_MEM_FLAGS_HOST_PTR_USE at step
3, while it was assumed otherwise during the release of this allocation.

Signed-off-by: Sergey Semenov [email protected]

Previously, whenever a native memory object was created, the original host
pointer passed by the user was checked for being read-only, regardless
of what host pointer was actually used at that point. This patch fixes the
issue by assuming a writeable host pointer unless it's the one provided by
the user.

As a result, this fixes a double free issue that appeared with the
following usage pattern:
  1. Create a buffer with a const user pointer
  2. Write-access it on host
  3. Access it on a non-host device
  4. Access it on host

This error was caused by the allocation performed at step 2 being
treated as read-only and, therefore, used with
PI_MEM_FLAGS_HOST_PTR_COPY rather than PI_MEM_FLAGS_HOST_PTR_USE at step
3, while it was assumed otherwise during the release of this allocation.

Signed-off-by: Sergey Semenov <[email protected]>
@sergey-semenov sergey-semenov requested a review from a team as a code owner October 28, 2020 12:10
Copy link
Contributor

@romanovvlad romanovvlad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@romanovvlad
Copy link
Contributor

/summary:run

@romanovvlad romanovvlad merged commit 8b55062 into intel:sycl Nov 3, 2020
@sergey-semenov sergey-semenov deleted the fixreadonlycheck branch November 3, 2020 08:56
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Nov 6, 2020
* upstream/sycl:
  [SYCL] Move tests with dependencies to on-device directory (intel#2732)
  [SYCL][Test] Remove leftovers for FPGA archives (intel#2735)
  [SYCL][NFC] Extend ABI tests to cover device code (intel#2725)
  [SYCL] Fix link to ESIMD tests (intel#2736)
  Added the SYCL_INTEL_mem_channel_property extension specification (intel#2688)
  [SYCL] Add support for new FPGA loop attribute nofusion (intel#2715)
  [SYCL] Remove host-task-dependency test added to llvm-test-suite (intel#2720)
  [SYCL] Remove warning about SYCL_EXTERNAL with pointers (intel#2722)
  [SYCL] dot_product support. (intel#2609)
  [SYCL][PI][L0] Fix a problem with kernels and programs destruction while they can be used (intel#2710)
  [SYCL] Fix the check for read-only host pointer during memobj creation (intel#2697)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants