[PATCH] kobject: Fix slab-out-of-bounds in fill_kobj_path()

From: Wang Hai
Date: Mon Dec 19 2022 - 09:42:12 EST


In kobject_get_path(), if kobj->name is changed between calls
get_kobj_path_length() and fill_kobj_path() and the length becomes
longer, then fill_kobj_path() will have an out-of-bounds bug.

The actual current problem occurs when the ixgbe probe.

In ixgbe_mii_bus_init(), if the length of netdev->dev.kobj.name
length becomes longer, out-of-bounds will occur.

cpu0 cpu1
ixgbe_probe
register_netdev(netdev)
netdev_register_kobject
device_add
kobject_uevent // Sending ADD events
systemd-udevd // rename netdev
dev_change_name
device_rename
kobject_rename
|
ixgbe_mii_bus_init |
mdiobus_register |
__mdiobus_register |
device_register |
device_add |
kobject_uevent |
kobject_get_path |
len = get_kobj_path_length // old name |
path = kzalloc(len, gfp_mask); |
kobj->name = name;
/* name length becomes
* longer
*/
fill_kobj_path /* kobj path length is
* longer than path,
* resulting in out of
* bounds when filling path
*/

This is the kasan report:

==================================================================
BUG: KASAN: slab-out-of-bounds in fill_kobj_path+0x50/0xc0
Write of size 7 at addr ff1100090573d1fd by task kworker/28:1/673

Workqueue: events work_for_cpu_fn
Call Trace:
<TASK>
dump_stack_lvl+0x34/0x48
print_address_description.constprop.0+0x86/0x1e7
print_report+0x36/0x4f
kasan_report+0xad/0x130
kasan_check_range+0x35/0x1c0
memcpy+0x39/0x60
fill_kobj_path+0x50/0xc0
kobject_get_path+0x5a/0xc0
kobject_uevent_env+0x140/0x460
device_add+0x5c7/0x910
__mdiobus_register+0x14e/0x490
ixgbe_probe.cold+0x441/0x574 [ixgbe]
local_pci_probe+0x78/0xc0
work_for_cpu_fn+0x26/0x40
process_one_work+0x3b6/0x6a0
worker_thread+0x368/0x520
kthread+0x165/0x1a0
ret_from_fork+0x1f/0x30

This reproducer triggers that bug:

while:
do
rmmod ixgbe
sleep 0.5
modprobe ixgbe
sleep 0.5

When calling fill_kobj_path() to fill path, if the name length of
kobj becomes longer, return failure and retry.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Wang Hai <wanghai38@xxxxxxxxxx>
---
lib/kobject.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index a0b2dbfcfa23..d129f437b200 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -112,7 +112,7 @@ static int get_kobj_path_length(struct kobject *kobj)
return length;
}

-static void fill_kobj_path(struct kobject *kobj, char *path, int length)
+static bool fill_kobj_path(struct kobject *kobj, char *path, int length)
{
struct kobject *parent;

@@ -121,12 +121,16 @@ static void fill_kobj_path(struct kobject *kobj, char *path, int length)
int cur = strlen(kobject_name(parent));
/* back up enough to print this name with '/' */
length -= cur;
+ if (length < 0)
+ return false;
memcpy(path + length, kobject_name(parent), cur);
*(path + --length) = '/';
}

pr_debug("kobject: '%s' (%p): %s: path = '%s'\n", kobject_name(kobj),
kobj, __func__, path);
+
+ return true;
}

/**
@@ -140,14 +144,20 @@ char *kobject_get_path(struct kobject *kobj, gfp_t gfp_mask)
{
char *path;
int len;
+ bool ret;

+retry:
len = get_kobj_path_length(kobj);
if (len == 0)
return NULL;
path = kzalloc(len, gfp_mask);
if (!path)
return NULL;
- fill_kobj_path(kobj, path, len);
+ ret = fill_kobj_path(kobj, path, len);
+ if (!ret) {
+ kfree(path);
+ goto retry;
+ }

return path;
}
--
2.17.1