Re: [PATCH] dma-buf/dma-fence: Set better string for dma_fence_driver_name()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] ABI breakage in UAPI `sync_fence_info` and severe string truncation.

Pre-existing issues:
- [Critical] Potential NULL pointer dereference due to unordered reads of `fence->ops` and `fence->flags`.
--

commit 2d4e0eac87d76c56e73514e9ac37b9db142b0e4f
Author: Philipp Stanner <phasta@kernel.org>

dma-buf/dma-fence: Set better string for dma_fence_driver_name()

Rename the "detached-driver" string returned by dma_fence_driver_name()
to something more descriptive for signaled fences.

> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index b3bfa6943a8e1..4a326d2ba7d19 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -1171,7 +1171,7 @@ const char __rcu *dma_fence_driver_name(struct dma_fence *fence)
>  	if (!dma_fence_test_signaled_flag(fence))
>  		return (const char __rcu *)ops->get_driver_name(fence);

[Severity: Critical]
This is a pre-existing issue, but could this cause a NULL pointer dereference?

In dma_fence_driver_name(), fence->ops and fence->flags are read
independently without an smp_rmb() memory barrier. If the CPU reads
fence->flags as unsignaled first, and concurrently another CPU signals the
fence, dma_fence_signal_timestamp_locked() can set fence->flags to signaled
and then set fence->ops to NULL (for fences lacking .release and .wait ops,
such as virtgpu_fence or v3d_fence).

The reader CPU might then read fence->ops as NULL, take the if branch (using
the cached flags=0), and crash on ops->get_driver_name(fence).

>  	else
> -		return (const char __rcu *)"detached-driver";
> +		return (const char __rcu *)"driver-whose-fence-is-signaled";

[Severity: Critical]
Does this break the userspace ABI and cause string truncation?

The driver_name field in struct sync_fence_info is exposed to userspace via
the SYNC_IOC_FILE_INFO ioctl. Changing this string from "detached-driver" to
"driver-whose-fence-is-signaled" breaks userspace tools that rely on the
established string to identify decoupled fences.

Also, this formats the UAPI sync_file name into a 32-byte buffer in
sync_file_get_name():

drivers/dma-buf/sync_file.c:sync_file_get_name() {
    ...
	snprintf(buf, len, "%s-%s%llu-%lld",
			 rcu_dereference(driver),
			 rcu_dereference(timeline),
			 fence->context,
			 fence->seqno);
    ...
}

The new 30-character driver name consumes almost the entire 32-byte buffer,
which truncates the timeline name and all subsequent fence metadata.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623133457.1029880-2-phasta@kernel.org?part=1