Re: [PATCH 01/10] drm/xe: Guard configfs attribute reads in getters

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

Pre-existing issues:
- [High] The getters for ctx_restore batch buffers return pointers to configfs-managed memory (`cs`), but drop the configfs reference via `config_group_put` before returning to the caller. This creates a race condition where a concurrent `rmdir` on the configfs directory can free the device configuration memory before the caller finishes using the pointer, leading to a Use-After-Free.
--

--- Patch [1]: [PATCH 01/10] drm/xe: Guard configfs attribute reads in getters ---
commit 8e7623fc58ac40897d93827d5632245e67750e49
Author: Stuart Summers <[email protected]>

drm/xe: Guard configfs attribute reads in getters

The commit wraps reads of dev->config in configfs getters with a
scoped_guard() to prevent torn reads caused by concurrent stores.

> diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
> index 052cce9621611..0a85043a9dd01 100644
> --- a/drivers/gpu/drm/xe/xe_configfs.c
> +++ b/drivers/gpu/drm/xe/xe_configfs.c
[ ... ]
> @@ -1308,10 +1313,12 @@ u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev,
>  	if (!dev)
>  		return 0;
>  
> -	if (cs)
> -		*cs = dev->config.ctx_restore_mid_bb[class].cs;
> +	scoped_guard(mutex, &dev->lock) {
> +		if (cs)
> +			*cs = dev->config.ctx_restore_mid_bb[class].cs;
>  
> -	len = dev->config.ctx_restore_mid_bb[class].len;
> +		len = dev->config.ctx_restore_mid_bb[class].len;
> +	}
>  	config_group_put(&dev->group);
>  
>  	return len;

[Severity: High]
This is a pre-existing issue, but does dropping the configfs reference here
allow the memory backing the returned pointer to be freed before the caller
finishes using it?

If a concurrent rmdir on the configfs directory drops the group's reference
count to 0, xe_config_device_release() could free the `cs` buffer via kfree().
The caller could then execute a memcpy() using the now-freed pointer,
resulting in a use-after-free.

> @@ -1335,8 +1342,10 @@ u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev,
>  	if (!dev)
>  		return 0;
>  
> -	*cs = dev->config.ctx_restore_post_bb[class].cs;
> -	len = dev->config.ctx_restore_post_bb[class].len;
> +	scoped_guard(mutex, &dev->lock) {
> +		*cs = dev->config.ctx_restore_post_bb[class].cs;
> +		len = dev->config.ctx_restore_post_bb[class].len;
> +	}
>  	config_group_put(&dev->group);
>  
>  	return len;

[Severity: High]
This is also a pre-existing issue, but does the same use-after-free risk
apply here in xe_configfs_get_ctx_restore_post_bb()? The memory backing `cs`
could similarly be freed when config_group_put() drops the reference to the
configfs group before returning the pointer to the caller.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1