]> git.ipfire.org Git - thirdparty/kernel/stable.git/commitdiff
block: nbd: add sanity check for first_minor
authorPavel Skripkin <paskripkin@gmail.com>
Thu, 12 Aug 2021 09:15:01 +0000 (12:15 +0300)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 15 Sep 2021 08:00:34 +0000 (10:00 +0200)
[ Upstream commit b1a811633f7321cf1ae2bb76a66805b7720e44c9 ]

Syzbot hit WARNING in internal_create_group(). The problem was in
too big disk->first_minor.

disk->first_minor is initialized by value, which comes from userspace
and there wasn't any sanity checks about value correctness. It can cause
duplicate creation of sysfs files/links, because disk->first_minor will
be passed to MKDEV() which causes truncation to byte. Since maximum
minor value is 0xff, let's check if first_minor is correct minor number.

NOTE: the root case of the reported warning was in wrong error handling
in register_disk(), but we can avoid passing knowingly wrong values to
sysfs API, because sysfs error messages can confuse users. For example:
user passed 1048576 as index, but sysfs complains about duplicate
creation of /dev/block/43:0. It's not obvious how 1048576 becomes 0.
Log and reproducer for above example can be found on syzkaller bug
report page.

Link: https://syzkaller.appspot.com/bug?id=03c2ae9146416edf811958d5fd7acfab75b143d1
Fixes: b0d9111a2d53 ("nbd: use an idr to keep track of nbd devices")
Reported-by: syzbot+9937dc42271cd87d4b98@syzkaller.appspotmail.com
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
drivers/block/nbd.c

index 7384058c24d046b8fc02fb9762fb59928c14d98f..4acf5c6cb80d268e1ab0943d2bc14720b9a6c436 100644 (file)
@@ -1717,7 +1717,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->fops = &nbd_fops;
        disk->private_data = nbd;
        sprintf(disk->disk_name, "nbd%d", index);