| Message ID | 20221220012143.52141-1-wanghai38@huawei.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:adf:e747:0:0:0:0:0 with SMTP id c7csp2713407wrn;
Mon, 19 Dec 2022 17:28:25 -0800 (PST)
X-Google-Smtp-Source:
AA0mqf5+mHoChZ8QkgtOqBbcxfb+6aEmNzKaZK+zg2eVfmpNWt6i1S3iUHnzqNS2VON/P725OgmT
X-Received: by 2002:a17:906:828f:b0:7c1:652:d109 with SMTP id
h15-20020a170906828f00b007c10652d109mr38113960ejx.35.1671499704972;
Mon, 19 Dec 2022 17:28:24 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1671499704; cv=none;
d=google.com; s=arc-20160816;
b=TwrUmD1RStpgc8gLgLS4jfDd292PTY+ZRxPkK6eT1gF9R0gzA5GghY5xJ2jLUWHyWJ
F69En/ytyhWLPPb9r3W2Wo0FUahruBaBFgJVD8GBwOD3EbImTS59ES4J0JYb56Ey0yNP
TUa29tz7TmJHT4CygTf+sfVukykDJ9bnDmKe7ve2KrQK6MCi0ltlukuJwtQ16UTAF9zv
ZVlY8j07ev7MMBW525xuqfR+BIeBvrMFCZ1yHin0uPnhR+RrMgAFCxwE8wP72U9MU3Q9
35tFYcYcBXmeb0Rf4IHquSg535I08mXyOhEZG1RTBktWpiGy9QXdsk/IFS/totYir30C
Iiyw==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=list-id:precedence:mime-version:message-id:date:subject:cc:to:from;
bh=3d09prxq54KnjTcHMi0gRm/bHaMO6sNkJD412gjIhb0=;
b=maG2DpqWZ8WSvjOlZGqgsr65honksJ+xK81i2dXEgj7LuEhhNhTpRvbsywfNAcsfwo
oPhyjcgIAtWqZ7SfQBSdNy0UFX3uKHvqatuupHVNw9ts7Rh4n/3wuvBMqZeU0+3CeTfl
e93C7YMHHCMvLfYct8OmzcDqlnc6LjHRZZ+6L3ELwAza4PofMF00deru9Tey5xBIPLoI
/uK37BMaQNBZt87igLsWuq6rFuftClKPE0GOur6kTY6BTV6F814VaGuXmtHqyZM6I4OR
vhDMxq4lujrYCVb76vb1rTdG8m8WPLd+ZXPem5NQXme+csc9+BFkuf24EbOAImQZkvBi
ezGA==
ARC-Authentication-Results: i=1; mx.google.com;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
le6-20020a170907170600b007c1031bec6esi8961789ejc.857.2022.12.19.17.28.00;
Mon, 19 Dec 2022 17:28:24 -0800 (PST)
Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
client-ip=2620:137:e000::1:20;
Authentication-Results: mx.google.com;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S232432AbiLTB1A (ORCPT <rfc822;abdi.embedded@gmail.com>
+ 99 others); Mon, 19 Dec 2022 20:27:00 -0500
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52464 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S233255AbiLTBYc (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Mon, 19 Dec 2022 20:24:32 -0500
Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BDC2111A1E
for <linux-kernel@vger.kernel.org>;
Mon, 19 Dec 2022 17:22:34 -0800 (PST)
Received: from kwepemm600001.china.huawei.com (unknown [172.30.72.53])
by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4Nbf0z4gmLzRq6v;
Tue, 20 Dec 2022 09:21:23 +0800 (CST)
Received: from huawei.com (10.175.113.133) by kwepemm600001.china.huawei.com
(7.193.23.3) with Microsoft SMTP Server (version=TLS1_2,
cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.34; Tue, 20 Dec
2022 09:22:32 +0800
From: Wang Hai <wanghai38@huawei.com>
To: <gregkh@linuxfoundation.org>, <alice.chao@mediatek.com>,
<rafael@kernel.org>, <jesse.brandeburg@intel.com>,
<anthony.l.nguyen@intel.com>
CC: <intel-wired-lan@lists.osuosl.org>, <linux-kernel@vger.kernel.org>
Subject: [PATCH v2] kobject: Fix slab-out-of-bounds in fill_kobj_path()
Date: Tue, 20 Dec 2022 09:21:43 +0800
Message-ID: <20221220012143.52141-1-wanghai38@huawei.com>
X-Mailer: git-send-email 2.17.1
MIME-Version: 1.0
Content-Type: text/plain
X-Originating-IP: [10.175.113.133]
X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To
kwepemm600001.china.huawei.com (7.193.23.3)
X-CFilter-Loop: Reflected
X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED,
SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1752694474654490730?=
X-GMAIL-MSGID: =?utf-8?q?1752694474654490730?=
|
| Series |
[v2] kobject: Fix slab-out-of-bounds in fill_kobj_path()
|
|
Commit Message
Wang Hai
Dec. 20, 2022, 1:21 a.m. UTC
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. This fixes the problem.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Wang Hai <wanghai38@huawei.com>
---
v1->v2: Return value type change and some formatting adjustments.
lib/kobject.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
Comments
On Tue, Dec 20, 2022 at 09:21:43AM +0800, Wang Hai wrote: > 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. This fixes the problem. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Wang Hai <wanghai38@huawei.com> > --- > v1->v2: Return value type change and some formatting adjustments. > lib/kobject.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/lib/kobject.c b/lib/kobject.c > index a0b2dbfcfa23..3f97d903266a 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 int 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 -EINVAL; > 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 0; > } > > /** > @@ -141,13 +145,17 @@ char *kobject_get_path(struct kobject *kobj, gfp_t gfp_mask) > char *path; > int len; > > +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); > + if (fill_kobj_path(kobj, path, len)) { > + kfree(path); > + goto retry; > + } Thanks for the fix. I wonder if there is no case we end up with infinite loop (fill_kobj_path always returning error). Do You know? > > return path; > } > -- > 2.17.1 > > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
在 2022/12/21 19:08, Michal Swiatkowski 写道: > On Tue, Dec 20, 2022 at 09:21:43AM +0800, Wang Hai wrote: >> 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. This fixes the problem. >> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> Signed-off-by: Wang Hai <wanghai38@huawei.com> >> --- >> v1->v2: Return value type change and some formatting adjustments. >> lib/kobject.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/lib/kobject.c b/lib/kobject.c >> index a0b2dbfcfa23..3f97d903266a 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 int 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 -EINVAL; >> 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 0; >> } >> >> /** >> @@ -141,13 +145,17 @@ char *kobject_get_path(struct kobject *kobj, gfp_t gfp_mask) >> char *path; >> int len; >> >> +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); >> + if (fill_kobj_path(kobj, path, len)) { >> + kfree(path); >> + goto retry; >> + } > Thanks for the fix. > > I wonder if there is no case we end up with infinite loop > (fill_kobj_path always returning error). Do You know? It should only be possible to have an infinite loop if name or parent keeps changing. The probability of this is extremely low. If necessary, change it to only retry 3 times? >> >> return path; >> } >> -- >> 2.17.1 >> >> _______________________________________________ >> Intel-wired-lan mailing list >> Intel-wired-lan@osuosl.org >> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan > .
在 2022/12/20 9:21, Wang Hai 写道: > 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. This fixes the problem. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Wang Hai <wanghai38@huawei.com> > --- Hi, greg k-h. Sorry to bother you. Can this patch be merged into the mainline?
On Mon, Jan 09, 2023 at 05:37:23PM +0800, Wang Hai wrote: > > 在 2022/12/20 9:21, Wang Hai 写道: > > 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. This fixes the problem. > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > Signed-off-by: Wang Hai <wanghai38@huawei.com> > > --- > Hi, greg k-h. > Sorry to bother you. Can this patch be merged into the mainline? It's in my "to review" queue that I am working on. As this is not anything that a normal user can trigger, it's not that high of a priority, right? thanks, greg k-h
在 2023/1/9 18:33, Greg KH 写道: > On Mon, Jan 09, 2023 at 05:37:23PM +0800, Wang Hai wrote: >> 在 2022/12/20 9:21, Wang Hai 写道: >>> 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. This fixes the problem. >>> >>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >>> Signed-off-by: Wang Hai <wanghai38@huawei.com> >>> --- >> Hi, greg k-h. >> Sorry to bother you. Can this patch be merged into the mainline? > It's in my "to review" queue that I am working on. As this is not > anything that a normal user can trigger, it's not that high of a > priority, right? > > thanks, > > greg k-h > . Thanks, I thought you had forgotten about it. I hope I'm not disturbing you.
On Mon, Jan 09, 2023 at 07:15:05PM +0800, Wang Hai wrote:
> Thanks, I thought you had forgotten about it. I hope I'm not disturbing you.
No bother at all, it's now added to my tree, thanks!
greg k-h
diff --git a/lib/kobject.c b/lib/kobject.c index a0b2dbfcfa23..3f97d903266a 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 int 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 -EINVAL; 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 0; } /** @@ -141,13 +145,17 @@ char *kobject_get_path(struct kobject *kobj, gfp_t gfp_mask) char *path; int len; +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); + if (fill_kobj_path(kobj, path, len)) { + kfree(path); + goto retry; + } return path; }