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

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@xxxxxxxxx>

ses_intf_remove_enclosure() frees ses_dev and the page1/page2/page10
buffers it owns before calling enclosure_unregister().  Only
enclosure_unregister() tears down the component sysfs attributes -- which
drains any in-flight get or set access -- and repoints edev->cb at the null
callbacks, so between the frees and that call the attributes stay live over
freed memory.  A concurrent read or write of a component attribute then
dereferences the freed edev->scratch in ses_get_page2_descriptor() or
ses_show_id(): a use-after-free reachable while the enclosure is removed
(hot unplug or delete) and its sysfs is accessed.  The early
edev->scratch = NULL does not help -- ses_page2_supported() does not check
it, so it only turns the use-after-free into a NULL dereference.

Unregister first, then free.  Save the component scratch pointer while edev
is still alive (enclosure_unregister() drops the enclosure device), and
drop the now-redundant early scratch clear.

Fixes: 9927c68864e9 ("[SCSI] ses: add new Enclosure ULD")
Closes: https://sashiko.dev/#/patchset/20260706-b4-disp-29a05ca3-v1-1-49591f469f60@xxxxxxxxx?part=1
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Bryam Vargas <hexlabsecurity@xxxxxxxxx>
---
Reproduced with an in-kernel KASAN litmus modelling the removal ordering
and the sysfs accessor: freeing ses_dev before enclosure_unregister() and
then dereferencing the freed scratch reports slab-use-after-free in
ses_get_page2_descriptor() (the page2 pointer, 16 bytes into the kmalloc-64
ses_device) and in ses_show_id() (the page1 pointer, offset 0); reordering
so enclosure_unregister() runs first is clean, as is the single-threaded
case.  Because the freed ses_device is a kmalloc-64 pointer carrier, a
reclaim of the freed slot turns ses_show_id()'s page1 read into an
arbitrary-read oracle and ses_get_page2_descriptor() -> ses_recv_diag()
into an arbitrary write.

Triggerable while a component sysfs attribute (fault/status/locate/active/
id) is read or written concurrently with enclosure removal (hot unplug or a
delete); no malicious device required.
---
 drivers/scsi/ses.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 4c348645b04e..a3039a3ede56 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -862,6 +862,7 @@ static void ses_intf_remove_enclosure(struct scsi_device *sdev)
 {
 	struct enclosure_device *edev;
 	struct ses_device *ses_dev;
+	void *scomp;
 
 	/*  exact match to this enclosure */
 	edev = enclosure_find(&sdev->sdev_gendev, NULL);
@@ -869,18 +870,24 @@ static void ses_intf_remove_enclosure(struct scsi_device *sdev)
 		return;
 
 	ses_dev = edev->scratch;
-	edev->scratch = NULL;
+	scomp = edev->components ? edev->component[0].scratch : NULL;
+
+	/*
+	 * Unregister before freeing.  enclosure_unregister() tears down the
+	 * component sysfs attributes, draining any in-flight get or set access,
+	 * and points edev->cb at the null callbacks.  Freeing ses_dev and the
+	 * pages it references first leaves those attributes live over freed
+	 * memory, so a concurrent component-attribute access dereferences the
+	 * freed edev->scratch in ses_get_page2_descriptor() or ses_show_id().
+	 */
+	put_device(&edev->edev);
+	enclosure_unregister(edev);
 
 	kfree(ses_dev->page10);
 	kfree(ses_dev->page1);
 	kfree(ses_dev->page2);
 	kfree(ses_dev);
-
-	if (edev->components)
-		kfree(edev->component[0].scratch);
-
-	put_device(&edev->edev);
-	enclosure_unregister(edev);
+	kfree(scomp);
 }
 
 static void ses_intf_remove(struct device *cdev)

---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260706-b4-disp-cf600574-2a11b71951aa

Best regards,
-- 
Bryam Vargas <hexlabsecurity@xxxxxxxxx>