linux-kernel - Re: [syzbot] WARNING in internal_create

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 12 Aug 2021 00:37:08 +0300
From:   Pavel Skripkin <paskripkin@...il.com>
To:     Christoph Hellwig <hch@....de>, Hillf Danton <hdanton@...a.com>
Cc:     syzbot <syzbot+9937dc42271cd87d4b98@...kaller.appspotmail.com>,
        linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com,
        Eric Sandeen <sandeen@...hat.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [syzbot] WARNING in internal_create_group

On 7/21/21 7:30 AM, Christoph Hellwig wrote:
> On Wed, Jul 21, 2021 at 11:37:03AM +0800, Hillf Danton wrote:
>> On Tue, 20 Jul 2021 11:53:27 -0700
>> >syzbot has found a reproducer for the following issue on:
>> >
>> >HEAD commit:    8cae8cd89f05 seq_file: disallow extremely large seq buffer..
>> >git tree:       upstream
>> >console output: https://syzkaller.appspot.com/x/log.txt?x=116f92ec300000
>> >kernel config:  https://syzkaller.appspot.com/x/.config?x=7273c75708b55890
>> >dashboard link: https://syzkaller.appspot.com/bug?extid=9937dc42271cd87d4b98
>> >syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=15fc287c300000
>> >C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=178cbf6a300000
> 
> <snip>
> 
>> >WARNING: CPU: 0 PID: 8435 at fs/sysfs/group.c:116 internal_create_group+0x911/0xb20 fs/sysfs/group.c:116
> 
> <snip>
> 
>> The device_add(ddev) in register_disk() may fail but it proceeds to register
>> block queue even at the failure ... this falls in the class of known issue
>> given the comment line.
>> 
>>  * FIXME: error handling
>>  */
>> static void __device_add_disk(struct device *parent, struct gendisk *disk,
> 
> Yes, Luis is working on actually fixing this - but it requires changes
> to every single block driver.  How does a cap on the seq_buf size
> propagate here, though?
> 

Hi!

I've looked into this, and, I think, we can add sanity check for 
first_minor. If user will pass too big index (syzbot's repro passes 
1048576) this value will be shifted to part_shift and then truncated to 
byte in __device_add_disk() and assigned to dev->devt. User may be 
confused about why he passed 1048576, but sysfs warns about duplicate 
creation of /dev/block/43:0

So, these type of errors can be handled before passing wrong values to 
sysfs API like this:

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index c38317979f74..600e9bab5d43 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1725,7 +1725,17 @@ static int nbd_dev_add(int index)
  	refcount_set(&nbd->refs, 1);
  	INIT_LIST_HEAD(&nbd->list);
  	disk->major = NBD_MAJOR;
+
+	/* Too big first_minor can cause duplicate creation of
+	 * sysfs files/links, since first_minor will be truncated to
+	 * byte in __device_add_disk().
+	 */
  	disk->first_minor = index << part_shift;
+	if (disk->first_minor > 0xff) {
+		err = -EINVAL;
+		goto out_free_idr;
+	}
+
  	disk->minors = 1 << part_shift;
  	disk->fops = &nbd_fops;
  	disk->private_data = nbd;


What to do you think about it?


With regards,
Pavel Skripkin