linux-kernel - Re: [PATCH] scsi: sd: call device_del() if device_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 31 Mar 2022 20:14:52 +0800
From:   Wenchao Hao <haowenchao@...wei.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
CC:     <fmdefrancesco@...il.com>, <axboe@...nel.dk>, <jejb@...ux.ibm.com>,
        <linux-block@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-scsi@...r.kernel.org>, <martin.petersen@...cle.com>,
        <syzbot+f08c77040fa163a75a46@...kaller.appspotmail.com>,
        <syzkaller-bugs@...glegroups.com>, <linfeilong@...wei.com>
Subject: Re: [PATCH] scsi: sd: call device_del() if device_add_disk() fails

On 2022/3/31 13:41, Dan Carpenter wrote:
> On Thu, Mar 31, 2022 at 11:26:22AM -0400, 'Wenchao Hao' via syzkaller-bugs wrote:
>> I do not think it's necessary to call device_del() on this path. If the device
>> has been added, put_device() would delete it from sysfs. So the origin error
>> handle is ok with me.
>>
> 
> No.  The original is buggy and it was detected at runtime by syzbot.
> It's not static analysis, it is an actual bug found in testing.
> 
Yes, it's a bug, but the root reason is not we forget to call 
device_del(sdkp->disk_dev). It's because we did not cleanup gendisk.
The leak memory is allocated in elevator_init_mq(), we should clean
this memory via blk_cleanup_queue().

I summit a patch which would fix this memory leak:

https://lore.kernel.org/linux-scsi/20220401011018.1026553-1-haowenchao@huawei.com/T/#u

> The device_put() unwinds device_initialize().  The device_del() unwinds
> device_add().  Take a look at the comments to device_add() or take a
> look at how device_register/unregister() work.
> 

You may read the implement of put_device(), it is based on kobj_xxx.
If the kobj is still in sysfs, a cleanup would be performed.
And device_del() seems would not decrease the reference count of kobj,
the main aim is to make it invisibleto sysfs.