Bug bounty fix by saifuddin-xilinx · Pull Request #9953 · Xilinx/XRT

saifuddin-xilinx · 2026-08-02T19:44:57Z

Problem solved by the commit

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

How problem was solved, alternative solutions (if any) and why they were rejected

Risks (if any) associated the changes in the commit

What has been tested and how, request additional testing if necessary

Documentation impact (if any)

SWSPLAT-9261: Two unsafe strcpy() calls in cu_conf2info() copy data from an attacker-controlled XGQ ring buffer slot into fixed-size kernel stack arrays kname[64] and iname[64] with no length bound. conf->name (char[64]) is sourced from a kmalloc(512) buffer populated via cpy_fromio() from the hardware XGQ ring. If no null byte or ':' delimiter exists in the 484 bytes starting at the name field offset, strsep() returns the full span and strcpy overflows kname[64] by up to 420 bytes, corrupting the adjacent xgq pointer and stack return address. Fix by: - Copying conf->name into a local buffer with explicit null termination before calling strsep(), bounding the read to sizeof(conf->name) - Replacing strcpy() with strscpy() to bound the write to the destination buffer size CVE-2026-40678 Signed-off-by: Saifuddin Kaijar <saifuddin.kaijar@amd.com>

The GitHub PR diff API returns HTTP 406 when a pull request changes more than 300 files. stsoe/clang-tidy-review uses this API (via PyGithub) and propagates the exception unhandled, causing the whole "clangtidy" job to fail. Fix by: - Adding `continue-on-error: true` to the "Create clang-tidy review" step so the job passes even when the action fails due to an API limit. - Adding `if: steps.review.outcome == 'success'` to the upload step so the upload only runs when review artifacts were actually produced.

SWSPLAT-13906: ert_ctrl_abort_sync() was a stub returning true immediately without draining in-flight commands. When a client closes its DRM fd, xocl_destroy_client() calls kds_fini_client() which invokes abort_sync() expecting it to wait for submitted commands to complete before kfree(client) is called. The stub bypassed this, leaving a race where notify_execbuf() could dereference the freed xcmd->client pointer via hw_ctx_list traversal, atomic_inc(&client->event), and wake_up_interruptible(&client->waitq). Implement ert_ctrl_abort_sync() to sum submitted/completed counts across all hw contexts and wait up to 5 seconds for them to equalise, matching the drain behaviour in ert_user xocl_ert_user_abort_sync(). CVE-2026-43516 Signed-off-by: Saifuddin Kaijar <saifuddin.kaijar@amd.com>

SWSPLAT-9316: xocl_pwrite_unmgd_ioctl() and xocl_pread_unmgd_ioctl() passed caller-supplied physical addresses directly to xocl_migrate_unmgd() without any privilege check. The xocl_validate_paddr() guard was compiled out (#if 0) with no replacement, allowing any render-group user to read or write arbitrary physical memory including kernel text and data, enabling local privilege escalation. Add capable(CAP_SYS_ADMIN) checks to both IOCTLs, consistent with the pattern used by xocl_alloc_cma_ioctl() and xocl_free_cma_ioctl() which similarly expose privileged hardware operations. CVE-2026-43552 Signed-off-by: Saifuddin Kaijar <saifuddin.kaijar@amd.com>

SWSPLAT-9444: xocl_cu_ctx_to_info() and zocl_cu_ctx_to_info() use strcpy() to copy strsep() tokens from cu_args->cu_name (char[64]) into fixed-size stack buffers kname[64] and iname[64]. If cu_name contains no ':' delimiter and is not NUL-terminated within 64 bytes, strsep() reads past the field boundary and strcpy overwrites adjacent stack memory, causing kernel memory corruption and a system crash. Replace strcpy() with strscpy() and enforce NUL-termination of the source field before strsep(), matching the fix applied to cu_conf2info() in zocl_ctrl_ert.c (CVE-2026-40678). CVE-2026-43545 Signed-off-by: Saifuddin Kaijar <saifuddin.kaijar@amd.com>

Copilot

Pull request overview

This PR appears intended as a security hardening / “bug bounty” fix in the kernel drivers by (1) replacing unsafe strcpy + strsep parsing with bounded copies, (2) restricting unmanaged pwrite/pread ioctls to privileged callers, and (3) making the clang-tidy PR review workflow resilient to GitHub diff API 406 responses on very large diffs.

Changes:

Replace strcpy(strsep(...)) parsing with memcpy/strscpy in several CU name parsing paths.
Require CAP_SYS_ADMIN for unmanaged pwrite/pread ioctls.
Update clang-tidy review workflow to continue on review-step error and only upload when the review step succeeds.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/runtime_src/core/pcie/driver/linux/xocl/userpf/xocl_hwctx.c	Updates CU name parsing, but currently breaks PL CU matching / introduces a syntax issue in the CU lookup loop.
src/runtime_src/core/pcie/driver/linux/xocl/userpf/xocl_bo.c	Adds `CAP_SYS_ADMIN` gating, but leaves a duplicated/orphaned block that breaks compilation.
src/runtime_src/core/pcie/driver/linux/xocl/subdev/ert_ctrl.c	Reworks `ert_ctrl_abort_sync`, but the surrounding region appears corrupted/truncated (stray line + orphaned statements).
src/runtime_src/core/edge/drm/zocl/zert/zocl_ctrl_ert.c	Updates CU name parsing to bounded copies for ERT CU config.
src/runtime_src/core/edge/drm/zocl/edge/zocl_hwctx.c	Updates CU name parsing, but currently removes PL CU matching logic (PL CUs can’t be found).
.github/workflows/clangtidy-review.yml	Makes clang-tidy review step non-fatal and guards upload on successful review generation.

Suppressed comments (1)

src/runtime_src/core/pcie/driver/linux/xocl/userpf/xocl_bo.c:1533

After the new early-return implementation of xocl_pread_unmgd_ioctl, an old copy of the function body remains starting at the comment on line 1520. This code is now outside any function (references args, xdev, ret out of scope) and also duplicates the migrate/return, which will break compilation. Remove the leftover block entirely.

	/* currently we are not able to return error because
	 * it is unclear that what addresses are valid other than
	 * ddr area. we should revisit this sometime.
	 * if (!xocl_validate_paddr(xdev, args->paddr, args->size)) {
	 *	userpf_err(xdev, "invalid paddr: 0x%llx, size:0x%llx",
	 *		args->paddr, args->size);
	 *	return -EINVAL;
	 * }
	 */

	ret = xocl_migrate_unmgd(xdev, args->data_ptr, args->paddr, args->size, 0);

	return ret;
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

        /* Retrieve the CU index from the given slot */
        for (i = 0; i < MAX_CUS; i++) {
                xcu = kds->cu_mgmt.xcus[i];
                if (!xcu)
                        continue;
-
-                if ((xcu->info.slot_idx == slot_hndl) &&
-                                (!strcmp(xcu->info.kname, kname)) &&
-                                (!strcmp(xcu->info.iname, iname))) {
-                        cu_info->cu_domain = DOMAIN_PL;
-                        cu_info->cu_idx = i;
                        goto done;
                }
        }


    /* Retrive the CU index from the given slot */
    for (i = 0; i < MAX_CUS; i++) {
        xcu = kds->cu_mgmt.xcus[i];
        if (!xcu)
            continue;
-
-        if ((xcu->info.slot_idx == slot_hndl) && (!strcmp(xcu->info.kname, kname)) && (!strcmp(xcu->info.iname, iname))) {
-            kds_cu_info->cu_domain = DOMAIN_PL;
-            kds_cu_info->cu_idx = i;
-            goto done;
-        }
    }


+	return true;
+}
+537
 	subdev_info.priv_data = &priv;
 	subdev_info.data_len = sizeof(priv);


github-actions · 2026-08-02T19:48:47Z