lkml.org 
[lkml]   [2022]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 1/1] scsi: Add length check to prevent invalid memory access
    Date
    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@mediatek.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
    \
     
     \ /
      Last update: 2022-12-16 04:17    [W:2.195 / U:0.324 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site