KASAN: use-after-free Read in bdev_del_partition

KASAN: use-after-free Read in bdev_del_partition

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    dcc5c6f0 Merge tag 'x86-urgent-2020-08-30' of git://git.ke..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11e8ca85900000
kernel config:  https://syzkaller.appspot.com/x/.config?x=891ca5711a9f1650
dashboard link: https://syzkaller.appspot.com/bug?extid=6448f3c229bc52b82f69
compiler:       clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=154e2285900000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16f6e915900000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+6448f3c229bc52b82f69@syzkaller.appspotmail.com

==================================================================
BUG: KASAN: use-after-free in kobject_put+0x245/0x2c0 lib/kobject.c:748
Read of size 1 at addr ffff8880a384e9bc by task syz-executor027/6982

CPU: 0 PID: 6982 Comm: syz-executor027 Not tainted 5.9.0-rc2-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x1d6/0x29e lib/dump_stack.c:118
 print_address_description+0x66/0x620 mm/kasan/report.c:383
 __kasan_report mm/kasan/report.c:513 [inline]
 kasan_report+0x132/0x1d0 mm/kasan/report.c:530
 kobject_put+0x245/0x2c0 lib/kobject.c:748
 delete_partition block/partitions/core.c:324 [inline]
 bdev_del_partition+0x26a/0x380 block/partitions/core.c:549
 blkpg_do_ioctl+0x12b/0x2e0 block/ioctl.c:33
 blkpg_ioctl block/ioctl.c:69 [inline]
 blkdev_ioctl+0x352/0x5d0 block/ioctl.c:589
 block_ioctl+0xbc/0x100 fs/block_dev.c:1871
 vfs_ioctl fs/ioctl.c:48 [inline]
 __do_sys_ioctl fs/ioctl.c:753 [inline]
 __se_sys_ioctl+0xfb/0x170 fs/ioctl.c:739
 do_syscall_64+0x31/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x446959
Code: e8 0c e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 db 06 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f4a7a555d98 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000006dbc38 RCX: 0000000000446959
RDX: 0000000020000000 RSI: 0000000000001269 RDI: 0000000000000005
RBP: 00000000006dbc30 R08: 00007f4a7a556700 R09: 0000000000000000
R10: 00007f4a7a556700 R11: 0000000000000246 R12: 00000000006dbc3c
R13: 00000000000000a9 R14: 00b747111e42e3ec R15: 02ba2b7a04b8ac00

Allocated by task 6977:
 kasan_save_stack mm/kasan/common.c:48 [inline]
 kasan_set_track mm/kasan/common.c:56 [inline]
 __kasan_kmalloc+0x100/0x130 mm/kasan/common.c:461
 kmem_cache_alloc_trace+0x1e4/0x2e0 mm/slab.c:3550
 kmalloc include/linux/slab.h:554 [inline]
 kzalloc include/linux/slab.h:666 [inline]
 kobject_create lib/kobject.c:783 [inline]
 kobject_create_and_add+0x57/0x240 lib/kobject.c:809
 add_partition+0x884/0xcb0 block/partitions/core.c:443
 bdev_add_partition+0x1c9/0x230 block/partitions/core.c:518
 blkpg_do_ioctl+0x295/0x2e0 block/ioctl.c:52
 blkpg_ioctl block/ioctl.c:69 [inline]
 blkdev_ioctl+0x352/0x5d0 block/ioctl.c:589
 block_ioctl+0xbc/0x100 fs/block_dev.c:1871
 vfs_ioctl fs/ioctl.c:48 [inline]
 __do_sys_ioctl fs/ioctl.c:753 [inline]
 __se_sys_ioctl+0xfb/0x170 fs/ioctl.c:739
 do_syscall_64+0x31/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Freed by task 6979:
 kasan_save_stack mm/kasan/common.c:48 [inline]
 kasan_set_track+0x3d/0x70 mm/kasan/common.c:56
 kasan_set_free_info+0x17/0x30 mm/kasan/generic.c:355
 __kasan_slab_free+0xdd/0x110 mm/kasan/common.c:422
 __cache_free mm/slab.c:3418 [inline]
 kfree+0x113/0x200 mm/slab.c:3756
 kobject_cleanup lib/kobject.c:704 [inline]
 kobject_release lib/kobject.c:735 [inline]
 kref_put include/linux/kref.h:65 [inline]
 kobject_put+0x1a0/0x2c0 lib/kobject.c:752
 delete_partition block/partitions/core.c:324 [inline]
 bdev_del_partition+0x26a/0x380 block/partitions/core.c:549
 blkpg_do_ioctl+0x12b/0x2e0 block/ioctl.c:33
 blkpg_ioctl block/ioctl.c:69 [inline]
 blkdev_ioctl+0x352/0x5d0 block/ioctl.c:589
 block_ioctl+0xbc/0x100 fs/block_dev.c:1871
 vfs_ioctl fs/ioctl.c:48 [inline]
 __do_sys_ioctl fs/ioctl.c:753 [inline]
 __se_sys_ioctl+0xfb/0x170 fs/ioctl.c:739
 do_syscall_64+0x31/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

The buggy address belongs to the object at ffff8880a384e980
 which belongs to the cache kmalloc-64 of size 64
The buggy address is located 60 bytes inside of
 64-byte region [ffff8880a384e980, ffff8880a384e9c0)
The buggy address belongs to the page:
page:00000000f601aef4 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0xa384e
flags: 0xfffe0000000200(slab)
raw: 00fffe0000000200 ffffea000263b548 ffffea00029bbac8 ffff8880aa440200
raw: 0000000000000000 ffff8880a384e000 0000000100000020 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff8880a384e880: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
 ffff8880a384e900: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
>ffff8880a384e980: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
                                        ^
 ffff8880a384ea00: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
 ffff8880a384ea80: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
==================================================================


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

Re: KASAN: use-after-free Read in bdev_del_partition

[syzbot]

syzbot has bisected this issue to:

commit cddae808aeb77e5c29d22a8e0dfbdaed413f9e04
Author: Christoph Hellwig <hch@lst.de>
Date:   Tue Apr 14 07:28:54 2020 +0000

    block: pass a hd_struct to delete_partition

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=17fa7656900000
start commit:   dcc5c6f0 Merge tag 'x86-urgent-2020-08-30' of git://git.ke..
git tree:       upstream
final oops:     https://syzkaller.appspot.com/x/report.txt?x=14067656900000
console output: https://syzkaller.appspot.com/x/log.txt?x=10067656900000
kernel config:  https://syzkaller.appspot.com/x/.config?x=891ca5711a9f1650
dashboard link: https://syzkaller.appspot.com/bug?extid=6448f3c229bc52b82f69
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=154e2285900000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16f6e915900000

Reported-by: syzbot+6448f3c229bc52b82f69@syzkaller.appspotmail.com
Fixes: cddae808aeb7 ("block: pass a hd_struct to delete_partition")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

Re: KASAN: use-after-free Read in bdev_del_partition

[Peilin Ye]

Hi all,

I have sent a patch which reverts cddae808aeb7 ("block: pass a
hd_struct to delete_partition") to syzbot, and it is now pending for
testing:

Link: https://groups.google.com/g/syzkaller-bugs/c/fnk0t9aqhDw/m/FO46-S8XAgAJ

Thank you,
Peilin Ye

[Linux-kernel-mentees] [PATCH] block: Fix use-after-free in bdev_del_partition()

[Peilin Ye]

In bdev_del_partition(), `part` is being looked up outside the critical
section. This is causing bdev_del_partition() to delete the same partition
more than once. Fix it by reverting commit cddae808aeb7.

Fixes: cddae808aeb7 ("block: pass a hd_struct to delete_partition")
Reported-and-tested-by: syzbot+6448f3c229bc52b82f69@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Cc: Hillf Danton <hdanton@sina.com>
Link: https://syzkaller.appspot.com/bug?id=09fc5ec437ea150f28e8a19f5011c08ee73381af
Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
---
 block/blk.h             |  2 +-
 block/genhd.c           |  2 +-
 block/partitions/core.c | 22 ++++++++++++++--------
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index 49e2928a1632..f5a46d0020fb 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -350,7 +350,7 @@ char *disk_name(struct gendisk *hd, int partno, char *buf);
 #define ADDPART_FLAG_NONE	0
 #define ADDPART_FLAG_RAID	1
 #define ADDPART_FLAG_WHOLEDISK	2
-void delete_partition(struct gendisk *disk, struct hd_struct *part);
+void delete_partition(struct gendisk *disk, int partno);
 int bdev_add_partition(struct block_device *bdev, int partno,
 		sector_t start, sector_t length);
 int bdev_del_partition(struct block_device *bdev, int partno);
diff --git a/block/genhd.c b/block/genhd.c
index 99c64641c314..40abc70b92ab 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -913,7 +913,7 @@ void del_gendisk(struct gendisk *disk)
 			     DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE);
 	while ((part = disk_part_iter_next(&piter))) {
 		invalidate_partition(disk, part->partno);
-		delete_partition(disk, part);
+		delete_partition(disk, part->partno);
 	}
 	disk_part_iter_exit(&piter);
 
diff --git a/block/partitions/core.c b/block/partitions/core.c
index e62a98a8eeb7..1fe1fe593423 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -310,17 +310,25 @@ int hd_ref_init(struct hd_struct *part)
  * Must be called either with bd_mutex held, before a disk can be opened or
  * after all disk users are gone.
  */
-void delete_partition(struct gendisk *disk, struct hd_struct *part)
+void delete_partition(struct gendisk *disk, int partno)
 {
 	struct disk_part_tbl *ptbl =
 		rcu_dereference_protected(disk->part_tbl, 1);
+	struct hd_struct *part;
+
+	if (partno >= ptbl->len)
+		return;
+
+	part = rcu_dereference_protected(ptbl->part[partno], 1);
+	if (!part)
+		return;
 
 	/*
 	 * ->part_tbl is referenced in this part's release handler, so
 	 *  we have to hold the disk device
 	 */
 	get_device(disk_to_dev(part_to_disk(part)));
-	rcu_assign_pointer(ptbl->part[part->partno], NULL);
+	rcu_assign_pointer(ptbl->part[partno], NULL);
 	kobject_put(part->holder_dir);
 	device_del(part_to_dev(part));
 
@@ -531,10 +539,10 @@ int bdev_del_partition(struct block_device *bdev, int partno)
 	if (!part)
 		return -ENXIO;
 
-	ret = -ENOMEM;
 	bdevp = bdget(part_devt(part));
+	disk_put_part(part);
 	if (!bdevp)
-		goto out_put_part;
+		return -ENOMEM;
 
 	mutex_lock(&bdevp->bd_mutex);
 
@@ -546,15 +554,13 @@ int bdev_del_partition(struct block_device *bdev, int partno)
 	invalidate_bdev(bdevp);
 
 	mutex_lock_nested(&bdev->bd_mutex, 1);
-	delete_partition(bdev->bd_disk, part);
+	delete_partition(bdev->bd_disk, partno);
 	mutex_unlock(&bdev->bd_mutex);
 
 	ret = 0;
 out_unlock:
 	mutex_unlock(&bdevp->bd_mutex);
 	bdput(bdevp);
-out_put_part:
-	disk_put_part(part);
 	return ret;
 }
 
@@ -627,7 +633,7 @@ int blk_drop_partitions(struct block_device *bdev)
 
 	disk_part_iter_init(&piter, bdev->bd_disk, DISK_PITER_INCL_EMPTY);
 	while ((part = disk_part_iter_next(&piter)))
-		delete_partition(bdev->bd_disk, part);
+		delete_partition(bdev->bd_disk, part->partno);
 	disk_part_iter_exit(&piter);
 
 	return 0;
-- 
2.25.1

[Linux-kernel-mentees] [PATCH] block: Fix use-after-free in bdev_del_partition()

[Peilin Ye]

In bdev_del_partition(), `part` is being looked up outside the critical
section. This is causing bdev_del_partition() to delete the same partition
more than once. Fix it by reverting commit cddae808aeb7.

Fixes: cddae808aeb7 ("block: pass a hd_struct to delete_partition")
Reported-and-tested-by: syzbot+6448f3c229bc52b82f69@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Cc: Hillf Danton <hdanton@sina.com>
Link: https://syzkaller.appspot.com/bug?id=09fc5ec437ea150f28e8a19f5011c08ee73381af
Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
---
 block/blk.h             |  2 +-
 block/genhd.c           |  2 +-
 block/partitions/core.c | 22 ++++++++++++++--------
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index 49e2928a1632..f5a46d0020fb 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -350,7 +350,7 @@ char *disk_name(struct gendisk *hd, int partno, char *buf);
 #define ADDPART_FLAG_NONE	0
 #define ADDPART_FLAG_RAID	1
 #define ADDPART_FLAG_WHOLEDISK	2
-void delete_partition(struct gendisk *disk, struct hd_struct *part);
+void delete_partition(struct gendisk *disk, int partno);
 int bdev_add_partition(struct block_device *bdev, int partno,
 		sector_t start, sector_t length);
 int bdev_del_partition(struct block_device *bdev, int partno);
diff --git a/block/genhd.c b/block/genhd.c
index 99c64641c314..40abc70b92ab 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -913,7 +913,7 @@ void del_gendisk(struct gendisk *disk)
 			     DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE);
 	while ((part = disk_part_iter_next(&piter))) {
 		invalidate_partition(disk, part->partno);
-		delete_partition(disk, part);
+		delete_partition(disk, part->partno);
 	}
 	disk_part_iter_exit(&piter);
 
diff --git a/block/partitions/core.c b/block/partitions/core.c
index e62a98a8eeb7..1fe1fe593423 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -310,17 +310,25 @@ int hd_ref_init(struct hd_struct *part)
  * Must be called either with bd_mutex held, before a disk can be opened or
  * after all disk users are gone.
  */
-void delete_partition(struct gendisk *disk, struct hd_struct *part)
+void delete_partition(struct gendisk *disk, int partno)
 {
 	struct disk_part_tbl *ptbl =
 		rcu_dereference_protected(disk->part_tbl, 1);
+	struct hd_struct *part;
+
+	if (partno >= ptbl->len)
+		return;
+
+	part = rcu_dereference_protected(ptbl->part[partno], 1);
+	if (!part)
+		return;
 
 	/*
 	 * ->part_tbl is referenced in this part's release handler, so
 	 *  we have to hold the disk device
 	 */
 	get_device(disk_to_dev(part_to_disk(part)));
-	rcu_assign_pointer(ptbl->part[part->partno], NULL);
+	rcu_assign_pointer(ptbl->part[partno], NULL);
 	kobject_put(part->holder_dir);
 	device_del(part_to_dev(part));
 
@@ -531,10 +539,10 @@ int bdev_del_partition(struct block_device *bdev, int partno)
 	if (!part)
 		return -ENXIO;
 
-	ret = -ENOMEM;
 	bdevp = bdget(part_devt(part));
+	disk_put_part(part);
 	if (!bdevp)
-		goto out_put_part;
+		return -ENOMEM;
 
 	mutex_lock(&bdevp->bd_mutex);
 
@@ -546,15 +554,13 @@ int bdev_del_partition(struct block_device *bdev, int partno)
 	invalidate_bdev(bdevp);
 
 	mutex_lock_nested(&bdev->bd_mutex, 1);
-	delete_partition(bdev->bd_disk, part);
+	delete_partition(bdev->bd_disk, partno);
 	mutex_unlock(&bdev->bd_mutex);
 
 	ret = 0;
 out_unlock:
 	mutex_unlock(&bdevp->bd_mutex);
 	bdput(bdevp);
-out_put_part:
-	disk_put_part(part);
 	return ret;
 }
 
@@ -627,7 +633,7 @@ int blk_drop_partitions(struct block_device *bdev)
 
 	disk_part_iter_init(&piter, bdev->bd_disk, DISK_PITER_INCL_EMPTY);
 	while ((part = disk_part_iter_next(&piter)))
-		delete_partition(bdev->bd_disk, part);
+		delete_partition(bdev->bd_disk, part->partno);
 	disk_part_iter_exit(&piter);
 
 	return 0;
-- 
2.25.1

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH] block: Fix use-after-free in bdev_del_partition()

[Christoph Hellwig]

On Thu, Sep 03, 2020 at 02:55:34AM -0400, Peilin Ye wrote:
> In bdev_del_partition(), `part` is being looked up outside the critical
> section. This is causing bdev_del_partition() to delete the same partition
> more than once. Fix it by reverting commit cddae808aeb7.

We've already fix the problem properly:

https://git.kernel.dk/cgit/linux-block/commit/?h=block-5.9&id=08fc1ab6d748ab1a690fd483f41e2938984ce353

Re: [Linux-kernel-mentees] [PATCH] block: Fix use-after-free in bdev_del_partition()

[Christoph Hellwig]

On Thu, Sep 03, 2020 at 02:55:34AM -0400, Peilin Ye wrote:
> In bdev_del_partition(), `part` is being looked up outside the critical
> section. This is causing bdev_del_partition() to delete the same partition
> more than once. Fix it by reverting commit cddae808aeb7.

We've already fix the problem properly:

https://git.kernel.dk/cgit/linux-block/commit/?h=block-5.9&id=08fc1ab6d748ab1a690fd483f41e2938984ce353
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH] block: Fix use-after-free in bdev_del_partition()

[Peilin Ye]

On Thu, Sep 03, 2020 at 08:59:50AM +0200, Christoph Hellwig wrote:
> On Thu, Sep 03, 2020 at 02:55:34AM -0400, Peilin Ye wrote:
> > In bdev_del_partition(), `part` is being looked up outside the critical
> > section. This is causing bdev_del_partition() to delete the same partition
> > more than once. Fix it by reverting commit cddae808aeb7.
> 
> We've already fix the problem properly:
> 
> https://git.kernel.dk/cgit/linux-block/commit/?h=block-5.9&id=08fc1ab6d748ab1a690fd483f41e2938984ce353

Ah, I searched linux-kernel but didn't see your patch, sorry about
that. I should have searched linux-block too.

Thank you,
Peilin

Re: [Linux-kernel-mentees] [PATCH] block: Fix use-after-free in bdev_del_partition()

[Peilin Ye]

On Thu, Sep 03, 2020 at 08:59:50AM +0200, Christoph Hellwig wrote:
> On Thu, Sep 03, 2020 at 02:55:34AM -0400, Peilin Ye wrote:
> > In bdev_del_partition(), `part` is being looked up outside the critical
> > section. This is causing bdev_del_partition() to delete the same partition
> > more than once. Fix it by reverting commit cddae808aeb7.
> 
> We've already fix the problem properly:
> 
> https://git.kernel.dk/cgit/linux-block/commit/?h=block-5.9&id=08fc1ab6d748ab1a690fd483f41e2938984ce353

Ah, I searched linux-kernel but didn't see your patch, sorry about
that. I should have searched linux-block too.

Thank you,
Peilin
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees