Re: [PATCH] scsi: ses: unregister the enclosure before freeing its device state

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The enclosure device is published to sysfs and the global list before its private data pointers are initialized, allowing concurrent accesses to trigger a NULL pointer dereference.
--

commit c0db7f74419376e5dfdb039e9a143c3b6a8d01bf
Author: Bryam Vargas <hexlabsecurity@proton.me>

scsi: ses: unregister the enclosure before freeing its device state

ses_intf_remove_enclosure() frees ses_dev and the page1/page2/page10
buffers it owns before calling enclosure_unregister(). This leaves the
component sysfs attributes live over freed memory, meaning a concurrent
read or write could trigger a use-after-free. This patch reorders the
teardown to unregister first, then free.

Fixes: 9927c68864e9 ("[SCSI] ses: add new Enclosure ULD")
Closes: https://sashiko.dev/#/patchset/20260706-b4-disp-29a05ca3-v1-1-49591f469f60@proton.me?part=1
Cc: stable@vger.kernel.org

> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index 4c348645b04ee..a3039a3ede56c 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -869,18 +870,24 @@ static void ses_intf_remove_enclosure(struct scsi_device *sdev)
>  		return;
>  
>  	ses_dev = edev->scratch;
> -	edev->scratch = NULL;

[Severity: High]
This is a pre-existing issue, but since we are looking at sysfs race
conditions involving edev->scratch, could a similar race occur during
initialization in ses_intf_add()?

When ses_intf_add() registers the enclosure:

drivers/scsi/ses.c:ses_intf_add() {
	edev = enclosure_register(cdev->parent, dev_name(&sdev->sdev_gendev),
				  components, &ses_enclosure_callbacks);
	if (IS_ERR(edev)) {
		err = PTR_ERR(edev);
		goto err_free;
	}

	kfree(hdr_buf);

	edev->scratch = ses_dev;
	...
}

The enclosure_register() call makes the device visible to userspace and
could trigger a read of the id sysfs attribute before edev->scratch is
assigned.

If that happens, would the id_show callback lead to a NULL pointer
dereference here?

drivers/scsi/ses.c:ses_show_id() {
	struct ses_device *ses_dev = edev->scratch;
	unsigned long long id = get_unaligned_be64(ses_dev->page1+8+4);
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-b4-disp-cf600574-v1-1-be4f324ff44c@proton.me?part=1