Repository navigation
CR-1275329: zocl: reject aperture addresses that fall in System RAM - #10151
Open
bisingha-xilinx wants to merge 1 commit into
Open
bisingha-xilinx wants to merge 1 commit into
bisingha-xilinx wants to merge 1 commit into
Conversation
zocl_update_apertures() took the IP and debug IP m_base_address verbatim from a user supplied xclbin, and zocl_mmap() passed it to io_remap_pfn_range() after only checking the requested length against apt->size. Any caller of DRM_IOCTL_ZOCL_READ_AXLF, which is DRM_RENDER_ALLOW, could therefore map an arbitrary physical address read/write and uncached, with no bitstream download required. Add zocl_apt_range_is_mmio(), which rejects a range when the kernel reports System RAM in the pages a mapping would cover. Addresses the kernel knows nothing about stay allowed, since that is what the undeclared PL windows of real CUs look like. zocl_update_apertures() now skips an IP whose range fails the check, so a bad address never enters the table, and zocl_mmap() re-checks as a backstop. Also fixed: - zocl_xclbin_read_axlf() left a planted aperture behind when the load failed after the table was updated. A new out_apt label cleans the slot; the earlier paths keep out0 so the -EBUSY "xclbin is in-use" case does not wipe the previous xclbin. - apt_idx in zocl_mmap() was an int taking vma->vm_pgoff, so an offset above INT_MAX wrapped negative past the "< 0" test. - An aperture still holding EMPTY_APT_VALUE could reach io_remap_pfn_range(). Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Bikash Singha <bikash.singha@amd.com>
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem solved by the commit
zocl took CU and debug-IP base addresses verbatim from a user-supplied xclbin and
mapped them with
io_remap_pfn_range(), checking only the length. SinceREAD_AXLFisDRM_RENDER_ALLOWandrenderD*ships as 0666, any local processcould map arbitrary physical memory read/write.
Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered
CR-1275329. Not a recent regression - present since
4ed9af63c(#1582). Found byautomated security review, confirmed by hand.
How problem was solved, alternative solutions (if any) and why they were rejected
New
zocl_apt_range_is_mmio()rejects ranges the kernel reports as System RAM,enforced both when the aperture table is built and again in
zocl_mmap().Unknown addresses stay allowed, since that is what real PL windows look like.
Rejected a device-tree allow-list (new binding, breaks existing device trees) and
an mmap CU-context gate (cannot cover debug-IP apertures, which have no
cu_idx).Risks (if any) associated the changes in the commit
A false positive would fail the xclbin load. PL addresses on supported MPSoC and
Versal boards never overlap System RAM, so no effect there. Only
res_start != 0(U30, VCK5000) would be misjudged, and those are out of support 2026.2 onwards.
What has been tested and how, request additional testing if necessary