[PATCH] mcb: fix KASAN bug slab-use-after-free while removing mcb-pci

Jose Javier Rodriguez Barbarin posted 1 patch 1 month, 2 weeks ago
drivers/mcb/mcb-core.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
[PATCH] mcb: fix KASAN bug slab-use-after-free while removing mcb-pci
Posted by Jose Javier Rodriguez Barbarin 1 month, 2 weeks ago
Fix KASAN bug reported when removing mcb-pci module.

slab-use-after-free in mcb_release_bus drivers/mcb/mcb-core.c:73
Read of size 8 at addr ffff888101e52060 by task modprobe/4993

mcb_release_bus+0x3e/0x40 drivers/mcb/mcb-core.c:73
mcb_pci_remove+0x4e/0x70 [mcb_pci]
pci_device_remove+0xaa/0x1d0 drivers/pci/pci-driver.c:503
...

The bug is caused by an incorrect usage of bus_for_each_dev(), thus
if there are more than one pci mcb board, __mcb_bus_add_devices()
adds all bus devices recursively, so for mcb:0 it adds its devices
and for mcb:1 it adds the devices of mcb:0 and mcb:1.

The same behaviour is observed on __mcb_devices_unregister(). So
when unregistering process happens, for mcb:1, all devices are unregisted,
even the devices of mcb:0 so when mcb:0 tries to unregister its
devices, slab-use-after-free happens.

To fix that, just register/unregister the devices on the correct bus.
Additonally, with this change, mcb_bus have to be explicitly unregistered.

Reviewed-by: Filip Jensen <dev-Felipe.Jensen@duagon.com>
Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
---
 drivers/mcb/mcb-core.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/mcb/mcb-core.c b/drivers/mcb/mcb-core.c
index e6e7c11bac6c..13c30db803b2 100644
--- a/drivers/mcb/mcb-core.c
+++ b/drivers/mcb/mcb-core.c
@@ -308,13 +308,21 @@ EXPORT_SYMBOL_NS_GPL(mcb_alloc_bus, "MCB");
 
 static int __mcb_devices_unregister(struct device *dev, void *data)
 {
+	struct mcb_device *mdev = to_mcb_device(dev);
+	int bus_nr = *(int *)data;
+
+	if (mdev->bus->bus_nr != bus_nr)
+		return 0;
+
 	device_unregister(dev);
 	return 0;
 }
 
 static void mcb_devices_unregister(struct mcb_bus *bus)
 {
-	bus_for_each_dev(bus->dev.bus, NULL, NULL, __mcb_devices_unregister);
+	int bus_nr = bus->bus_nr;
+
+	bus_for_each_dev(bus->dev.bus, NULL, &bus_nr, __mcb_devices_unregister);
 }
 /**
  * mcb_release_bus() - Free a @mcb_bus
@@ -325,6 +333,7 @@ static void mcb_devices_unregister(struct mcb_bus *bus)
 void mcb_release_bus(struct mcb_bus *bus)
 {
 	mcb_devices_unregister(bus);
+	device_unregister(&bus->dev);
 }
 EXPORT_SYMBOL_NS_GPL(mcb_release_bus, "MCB");
 
@@ -390,8 +399,13 @@ EXPORT_SYMBOL_NS_GPL(mcb_free_dev, "MCB");
 
 static int __mcb_bus_add_devices(struct device *dev, void *data)
 {
+	struct mcb_device *mdev = to_mcb_device(dev);
+	int bus_nr = *(int *)data;
 	int retval;
 
+	if (mdev->bus->bus_nr != bus_nr)
+		return 0;
+
 	retval = device_attach(dev);
 	if (retval < 0) {
 		dev_err(dev, "Error adding device (%d)\n", retval);
@@ -409,7 +423,9 @@ static int __mcb_bus_add_devices(struct device *dev, void *data)
  */
 void mcb_bus_add_devices(const struct mcb_bus *bus)
 {
-	bus_for_each_dev(bus->dev.bus, NULL, NULL, __mcb_bus_add_devices);
+	int bus_nr = bus->bus_nr;
+
+	bus_for_each_dev(bus->dev.bus, NULL, &bus_nr, __mcb_bus_add_devices);
 }
 EXPORT_SYMBOL_NS_GPL(mcb_bus_add_devices, "MCB");
 
-- 
2.53.0
Re: [PATCH] mcb: fix KASAN bug slab-use-after-free while removing mcb-pci
Posted by Johannes Thumshirn 1 month ago
On 4/27/26 3:46 PM, Jose Javier Rodriguez Barbarin wrote:
> Fix KASAN bug reported when removing mcb-pci module.
>
> slab-use-after-free in mcb_release_bus drivers/mcb/mcb-core.c:73
> Read of size 8 at addr ffff888101e52060 by task modprobe/4993
>
> mcb_release_bus+0x3e/0x40 drivers/mcb/mcb-core.c:73
> mcb_pci_remove+0x4e/0x70 [mcb_pci]
> pci_device_remove+0xaa/0x1d0 drivers/pci/pci-driver.c:503
> ...
>
> The bug is caused by an incorrect usage of bus_for_each_dev(), thus
> if there are more than one pci mcb board, __mcb_bus_add_devices()
> adds all bus devices recursively, so for mcb:0 it adds its devices
> and for mcb:1 it adds the devices of mcb:0 and mcb:1.
>
> The same behaviour is observed on __mcb_devices_unregister(). So
> when unregistering process happens, for mcb:1, all devices are unregisted,
> even the devices of mcb:0 so when mcb:0 tries to unregister its
> devices, slab-use-after-free happens.
>
> To fix that, just register/unregister the devices on the correct bus.
> Additonally, with this change, mcb_bus have to be explicitly unregistered.
>
> Reviewed-by: Filip Jensen <dev-Felipe.Jensen@duagon.com>
> Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>


Looks good, thanks.