Re: [PATCH v4 5/6] drm/amdgpu/aca: Fix off-by-one buffer size parameter in add_aca_sysfs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Invalid free of an interior pointer causing a kernel panic during driver teardown.
--

--- Patch [5]: [PATCH v4 5/6] drm/amdgpu/aca: Fix off-by-one buffer size parameter in add_aca_sysfs ---
commit 4e00dff410196f38991a5223ac5b71f0384ad6fa
Author: Sreeraj S Kurup <sreekuttan2156239@gmail.com>

    drm/amdgpu/aca: Fix off-by-one buffer size parameter in add_aca_sysfs
    
    snprintf() guarantees NUL-termination within the size limit specified by
    its second argument. Passing 'sizeof(handle->attr_name) - 1'
    unnecessarily reduces the usable buffer capacity by one byte, causing
    sysfs attribute names to be truncated early.
    
    Pass 'sizeof(handle->attr_name)' directly to snprintf() in add_aca_sysfs()
    to allow full use of the allocated buffer space.
    
    Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@gmail.com>

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c
> index dcddeb3816285..3da3d369a9187 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c
> @@ -682,7 +682,7 @@ static int add_aca_sysfs(struct amdgpu_device *adev, struct aca_handle *handle)
>  {
>  	struct device_attribute *aca_attr = &handle->aca_attr;
>  
> -	snprintf(handle->attr_name, sizeof(handle->attr_name) - 1, "aca_%s", handle->name);
> +	snprintf(handle->attr_name, sizeof(handle->attr_name), "aca_%s", handle->name);

[Severity: High]
Is the handle pointer passed here an interior pointer that gets improperly
freed later?

Looking at amdgpu_ras_bind_aca(), it appears an embedded pointer is passed
into the ACA subsystem:

drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:amdgpu_ras_bind_aca() {
    ...
    return amdgpu_aca_add_handle(adev, &obj->aca_handle, ras_block_str(blk), aca_info, data);
}

During driver teardown, remove_aca_handle() unconditionally frees this
interior pointer:

drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c:remove_aca_handle() {
    ...
    kfree(handle);
}

Could this lead to a SLUB panic when removing the driver module or tearing
down the device?

>  	aca_attr->show = aca_sysfs_read;
>  	aca_attr->attr.name = handle->attr_name;
>  	aca_attr->attr.mode = S_IRUGO;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809084735.9743-1-sreekuttan2156239@gmail.com?part=5