[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221216031320.2634-1-alice.chao@mediatek.com>
Date: Fri, 16 Dec 2022 11:13:22 +0800
From: Alice Chao <alice.chao@...iatek.com>
To: <gregkh@...uxfoundation.org>, <rafael@...nel.org>,
<matthias.bgg@...il.com>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-mediatek@...ts.infradead.org>
CC: <stanley.chu@...iatek.com>, <peter.wang@...iatek.com>,
<chun-hung.wu@...iatek.com>, <alice.chao@...iatek.com>,
<powen.kao@...iatek.com>, <naomi.chu@...iatek.com>,
<cc.chou@...iatek.com>, <chaotian.jing@...iatek.com>,
<jiajie.hao@...iatek.com>, <qilin.tan@...iatek.com>,
<lin.gui@...iatek.com>, <yanxu.wei@...iatek.com>,
<tun-yu.yu@...iatek.com>, <eddie.huang@...iatek.com>,
<wsd_upstream@...iatek.com>
Subject: [PATCH 1/1] scsi: Add length check to prevent invalid memory access
Device reset thread uses kobject_uevent_env() to get kobj.parent(kobj.p)
, and it races with device init thread which calls device_add() to add
kobj.parent before kobject_uevent_env().
Device init call: Device reset call:
scsi_probe_and_add_lun() scsi_evt_thread()
scsi_add_lun() scsi_evt_emit()
scsi_sysfs_add_sdev() kobject_uevent_env() //get kobj.parent
scsi_target_add() kobject_get_path()
len = get_kobj_path_length ()
//len=1 because parent hasn't created yet
device_add() // add kobj.parent
kobject_uevent_env()
kobject_get_path() path = kzalloc()
fill_kobj_path() fill_kobj_path()
// --length; length -= cur is a negative value
memcpy(path + length, kobject_name(parent), cur);
// slab OOB!
Above backtrace describes the problem, device reset thread will get wrong
kobj.parent when device init thread didn't add kobj/parent yet. When this
racing happened, it triggers the a KASAN dump on the final iteration:
BUG: KASAN: slab-out-of-bounds in kobject_get_path+0xf8/0x1b8
Write of size 11 at addr ffffff80d6bb94f5 by task kworker/3:1/58
<snip>
Call trace:
__kasan_report+0x124/0x1c8
kasan_report+0x54/0x84
kasan_check_range+0x200/0x208
memcpy+0xb8/0xf0
kobject_get_path+0xf8/0x1b8
kobject_uevent_env+0x228/0xa88
scsi_evt_thread+0x2d0/0x5b0
process_one_work+0x570/0xf94
worker_thread+0x7cc/0xf80
kthread+0x2c4/0x388
These two jobs are scheduled asynchronously, we can't guaranteed that
kobj.parent will be created in device init thread before device reset
thread calls kobject_get_path().
To prevent length -= cur from being a negative value, we add length
check in fill_kobj_path() to prevent invalid memory access.
Signed-off-by: Alice Chao <alice.chao@...iatek.com>
---
lib/kobject.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/kobject.c b/lib/kobject.c
index af1f5f2954d4..3cccb8e88d4e 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -121,6 +121,10 @@ 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)
+ break;
+
memcpy(path + length, kobject_name(parent), cur);
*(path + --length) = '/';
}
--
2.18.0
Powered by blists - more mailing lists