public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] block: no show partitions if partno corrupted
       [not found] <67841058.050a0220.216c54.0034.GAE@google.com>
@ 2025-01-14  2:28 ` Edward Adam Davis
  2025-01-14  7:21   ` Hannes Reinecke
  2025-01-14  5:29 ` [syzbot] [fs?] KASAN: global-out-of-bounds Read in number syzbot
  1 sibling, 1 reply; 12+ messages in thread
From: Edward Adam Davis @ 2025-01-14  2:28 UTC (permalink / raw)
  To: syzbot+fcee6b76cf2e261c51a4
  Cc: axboe, linux-block, linux-kernel, syzkaller-bugs

syzbot reported a global-out-of-bounds in number. [1]

Corrupted partno causes out-of-bounds access when accessing the hex_asc_upper
array.

To avoid this issue, skip partitions with partno greater than DISK_MAX_PARTS.

[1]
BUG: KASAN: global-out-of-bounds in number+0x3be/0xf40 lib/vsprintf.c:494
Read of size 1 at addr ffffffff8c5fc971 by task syz-executor351/5832

CPU: 0 UID: 0 PID: 5832 Comm: syz-executor351 Not tainted 6.13.0-rc6-next-20250107-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:378 [inline]
 print_report+0x169/0x550 mm/kasan/report.c:489
 kasan_report+0x143/0x180 mm/kasan/report.c:602
 number+0x3be/0xf40 lib/vsprintf.c:494
 pointer+0x764/0x1210 lib/vsprintf.c:2484
 vsnprintf+0x75a/0x1220 lib/vsprintf.c:2846
 seq_vprintf fs/seq_file.c:391 [inline]
 seq_printf+0x172/0x270 fs/seq_file.c:406
 show_partition+0x29f/0x3f0 block/genhd.c:905
 seq_read_iter+0x969/0xd70 fs/seq_file.c:272
 proc_reg_read_iter+0x1c2/0x290 fs/proc/inode.c:299
 copy_splice_read+0x63a/0xb40 fs/splice.c:365
 do_splice_read fs/splice.c:985 [inline]
 splice_direct_to_actor+0x4af/0xc80 fs/splice.c:1089
 do_splice_direct_actor fs/splice.c:1207 [inline]
 do_splice_direct+0x289/0x3e0 fs/splice.c:1233
 do_sendfile+0x564/0x8a0 fs/read_write.c:1363
 __do_sys_sendfile64 fs/read_write.c:1424 [inline]
 __se_sys_sendfile64+0x17c/0x1e0 fs/read_write.c:1410
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Reported-by: syzbot+fcee6b76cf2e261c51a4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fcee6b76cf2e261c51a4
Tested-by: syzbot+fcee6b76cf2e261c51a4@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 block/genhd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/genhd.c b/block/genhd.c
index 9130e163e191..8d539a4a3b37 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -890,7 +890,9 @@ static int show_partition(struct seq_file *seqf, void *v)
 
 	rcu_read_lock();
 	xa_for_each(&sgp->part_tbl, idx, part) {
-		if (!bdev_nr_sectors(part))
+		int partno = bdev_partno(part);
+
+		if (!bdev_nr_sectors(part) || partno >= DISK_MAX_PARTS)
 			continue;
 		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
 			   MAJOR(part->bd_dev), MINOR(part->bd_dev),
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [syzbot] [fs?] KASAN: global-out-of-bounds Read in number
       [not found] <67841058.050a0220.216c54.0034.GAE@google.com>
  2025-01-14  2:28 ` [PATCH] block: no show partitions if partno corrupted Edward Adam Davis
@ 2025-01-14  5:29 ` syzbot
  1 sibling, 0 replies; 12+ messages in thread
From: syzbot @ 2025-01-14  5:29 UTC (permalink / raw)
  To: adobriyan, akpm, andriy.shevchenko, axboe, brauner, eadavis,
	kirill.shutemov, linux-block, linux-fsdevel, linux-kernel, linux,
	pmladek, rick.p.edgecombe, rostedt, senozhatsky, syzkaller-bugs,
	torvalds, viro, zhouchengming

syzbot has bisected this issue to:

commit 8d4826cc8a8aca01a3b5e95438dfc0eb3bd589ab
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu Dec 19 21:52:53 2024 +0000

    vsnprintf: collapse the number format state into one single state

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=16342a18580000
start commit:   7b4b9bf203da Add linux-next specific files for 20250107
git tree:       linux-next
final oops:     https://syzkaller.appspot.com/x/report.txt?x=15342a18580000
console output: https://syzkaller.appspot.com/x/log.txt?x=11342a18580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=63fa2c9d5e12faef
dashboard link: https://syzkaller.appspot.com/bug?extid=fcee6b76cf2e261c51a4
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=174f0a18580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=168aecb0580000

Reported-by: syzbot+fcee6b76cf2e261c51a4@syzkaller.appspotmail.com
Fixes: 8d4826cc8a8a ("vsnprintf: collapse the number format state into one single state")

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] block: no show partitions if partno corrupted
  2025-01-14  2:28 ` [PATCH] block: no show partitions if partno corrupted Edward Adam Davis
@ 2025-01-14  7:21   ` Hannes Reinecke
  2025-01-14  8:51     ` [PATCH V2] " Edward Adam Davis
  0 siblings, 1 reply; 12+ messages in thread
From: Hannes Reinecke @ 2025-01-14  7:21 UTC (permalink / raw)
  To: Edward Adam Davis, syzbot+fcee6b76cf2e261c51a4
  Cc: axboe, linux-block, linux-kernel, syzkaller-bugs

On 1/14/25 03:28, Edward Adam Davis wrote:
> syzbot reported a global-out-of-bounds in number. [1]
> 
> Corrupted partno causes out-of-bounds access when accessing the hex_asc_upper
> array.
> 
> To avoid this issue, skip partitions with partno greater than DISK_MAX_PARTS.
> 
> [1]
> BUG: KASAN: global-out-of-bounds in number+0x3be/0xf40 lib/vsprintf.c:494
> Read of size 1 at addr ffffffff8c5fc971 by task syz-executor351/5832
> 
> CPU: 0 UID: 0 PID: 5832 Comm: syz-executor351 Not tainted 6.13.0-rc6-next-20250107-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
> Call Trace:
>   <TASK>
>   __dump_stack lib/dump_stack.c:94 [inline]
>   dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
>   print_address_description mm/kasan/report.c:378 [inline]
>   print_report+0x169/0x550 mm/kasan/report.c:489
>   kasan_report+0x143/0x180 mm/kasan/report.c:602
>   number+0x3be/0xf40 lib/vsprintf.c:494
>   pointer+0x764/0x1210 lib/vsprintf.c:2484
>   vsnprintf+0x75a/0x1220 lib/vsprintf.c:2846
>   seq_vprintf fs/seq_file.c:391 [inline]
>   seq_printf+0x172/0x270 fs/seq_file.c:406
>   show_partition+0x29f/0x3f0 block/genhd.c:905
>   seq_read_iter+0x969/0xd70 fs/seq_file.c:272
>   proc_reg_read_iter+0x1c2/0x290 fs/proc/inode.c:299
>   copy_splice_read+0x63a/0xb40 fs/splice.c:365
>   do_splice_read fs/splice.c:985 [inline]
>   splice_direct_to_actor+0x4af/0xc80 fs/splice.c:1089
>   do_splice_direct_actor fs/splice.c:1207 [inline]
>   do_splice_direct+0x289/0x3e0 fs/splice.c:1233
>   do_sendfile+0x564/0x8a0 fs/read_write.c:1363
>   __do_sys_sendfile64 fs/read_write.c:1424 [inline]
>   __se_sys_sendfile64+0x17c/0x1e0 fs/read_write.c:1410
>   do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>   do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
>   entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> Reported-by: syzbot+fcee6b76cf2e261c51a4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=fcee6b76cf2e261c51a4
> Tested-by: syzbot+fcee6b76cf2e261c51a4@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>   block/genhd.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index 9130e163e191..8d539a4a3b37 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -890,7 +890,9 @@ static int show_partition(struct seq_file *seqf, void *v)
>   
>   	rcu_read_lock();
>   	xa_for_each(&sgp->part_tbl, idx, part) {
> -		if (!bdev_nr_sectors(part))
> +		int partno = bdev_partno(part);
> +
> +		if (!bdev_nr_sectors(part) || partno >= DISK_MAX_PARTS)
>   			continue;
>   		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
>   			   MAJOR(part->bd_dev), MINOR(part->bd_dev),
Maybe a warning is in order; when we are hitting this issue it means
that linux has a limitation on causing it to ignore the (otherwise 
valid) partition entry.

Otherwise looks good.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2] block: no show partitions if partno corrupted
  2025-01-14  7:21   ` Hannes Reinecke
@ 2025-01-14  8:51     ` Edward Adam Davis
  2025-01-14 14:16       ` Jens Axboe
  0 siblings, 1 reply; 12+ messages in thread
From: Edward Adam Davis @ 2025-01-14  8:51 UTC (permalink / raw)
  To: hare
  Cc: axboe, eadavis, linux-block, linux-kernel,
	syzbot+fcee6b76cf2e261c51a4, syzkaller-bugs

syzbot reported a global-out-of-bounds in number. [1]

Corrupted partno causes out-of-bounds access when accessing the hex_asc_upper
array.

To avoid this issue, skip partitions with partno greater than DISK_MAX_PARTS.

[1]
BUG: KASAN: global-out-of-bounds in number+0x3be/0xf40 lib/vsprintf.c:494
Read of size 1 at addr ffffffff8c5fc971 by task syz-executor351/5832

CPU: 0 UID: 0 PID: 5832 Comm: syz-executor351 Not tainted 6.13.0-rc6-next-20250107-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:378 [inline]
 print_report+0x169/0x550 mm/kasan/report.c:489
 kasan_report+0x143/0x180 mm/kasan/report.c:602
 number+0x3be/0xf40 lib/vsprintf.c:494
 pointer+0x764/0x1210 lib/vsprintf.c:2484
 vsnprintf+0x75a/0x1220 lib/vsprintf.c:2846
 seq_vprintf fs/seq_file.c:391 [inline]
 seq_printf+0x172/0x270 fs/seq_file.c:406
 show_partition+0x29f/0x3f0 block/genhd.c:905
 seq_read_iter+0x969/0xd70 fs/seq_file.c:272
 proc_reg_read_iter+0x1c2/0x290 fs/proc/inode.c:299
 copy_splice_read+0x63a/0xb40 fs/splice.c:365
 do_splice_read fs/splice.c:985 [inline]
 splice_direct_to_actor+0x4af/0xc80 fs/splice.c:1089
 do_splice_direct_actor fs/splice.c:1207 [inline]
 do_splice_direct+0x289/0x3e0 fs/splice.c:1233
 do_sendfile+0x564/0x8a0 fs/read_write.c:1363
 __do_sys_sendfile64 fs/read_write.c:1424 [inline]
 __se_sys_sendfile64+0x17c/0x1e0 fs/read_write.c:1410
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Reported-by: syzbot+fcee6b76cf2e261c51a4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fcee6b76cf2e261c51a4
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: Add a warning

 block/genhd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/genhd.c b/block/genhd.c
index 9130e163e191..8d539a4a3b37 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -890,7 +890,9 @@ static int show_partition(struct seq_file *seqf, void *v)
 
 	rcu_read_lock();
 	xa_for_each(&sgp->part_tbl, idx, part) {
-		if (!bdev_nr_sectors(part))
+		int partno = bdev_partno(part);
+
+		if (!bdev_nr_sectors(part) || WARN_ON(partno >= DISK_MAX_PARTS))
 			continue;
 		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
 			   MAJOR(part->bd_dev), MINOR(part->bd_dev),
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] block: no show partitions if partno corrupted
  2025-01-14  8:51     ` [PATCH V2] " Edward Adam Davis
@ 2025-01-14 14:16       ` Jens Axboe
  2025-01-14 14:58         ` [PATCH V3] " Edward Adam Davis
  2025-01-15  6:46         ` [PATCH V2] " Christoph Hellwig
  0 siblings, 2 replies; 12+ messages in thread
From: Jens Axboe @ 2025-01-14 14:16 UTC (permalink / raw)
  To: Edward Adam Davis, hare
  Cc: linux-block, linux-kernel, syzbot+fcee6b76cf2e261c51a4,
	syzkaller-bugs

On 1/14/25 1:51 AM, Edward Adam Davis wrote:
> diff --git a/block/genhd.c b/block/genhd.c
> index 9130e163e191..8d539a4a3b37 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -890,7 +890,9 @@ static int show_partition(struct seq_file *seqf, void *v)
>  
>  	rcu_read_lock();
>  	xa_for_each(&sgp->part_tbl, idx, part) {
> -		if (!bdev_nr_sectors(part))
> +		int partno = bdev_partno(part);
> +
> +		if (!bdev_nr_sectors(part) || WARN_ON(partno >= DISK_MAX_PARTS))
>  			continue;
>  		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
>  			   MAJOR(part->bd_dev), MINOR(part->bd_dev),

This should be a WARN_ON_ONCE(), and please put warn-on's on a separate
line.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V3] block: no show partitions if partno corrupted
  2025-01-14 14:16       ` Jens Axboe
@ 2025-01-14 14:58         ` Edward Adam Davis
  2025-01-14 15:02           ` Jens Axboe
  2025-01-15  6:46         ` [PATCH V2] " Christoph Hellwig
  1 sibling, 1 reply; 12+ messages in thread
From: Edward Adam Davis @ 2025-01-14 14:58 UTC (permalink / raw)
  To: axboe
  Cc: eadavis, hare, linux-block, linux-kernel,
	syzbot+fcee6b76cf2e261c51a4, syzkaller-bugs

syzbot reported a global-out-of-bounds in number. [1]

Corrupted partno causes out-of-bounds access when accessing the hex_asc_upper
array.

To avoid this issue, skip partitions with partno greater than DISK_MAX_PARTS.

[1]
BUG: KASAN: global-out-of-bounds in number+0x3be/0xf40 lib/vsprintf.c:494
Read of size 1 at addr ffffffff8c5fc971 by task syz-executor351/5832

CPU: 0 UID: 0 PID: 5832 Comm: syz-executor351 Not tainted 6.13.0-rc6-next-20250107-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:378 [inline]
 print_report+0x169/0x550 mm/kasan/report.c:489
 kasan_report+0x143/0x180 mm/kasan/report.c:602
 number+0x3be/0xf40 lib/vsprintf.c:494
 pointer+0x764/0x1210 lib/vsprintf.c:2484
 vsnprintf+0x75a/0x1220 lib/vsprintf.c:2846
 seq_vprintf fs/seq_file.c:391 [inline]
 seq_printf+0x172/0x270 fs/seq_file.c:406
 show_partition+0x29f/0x3f0 block/genhd.c:905
 seq_read_iter+0x969/0xd70 fs/seq_file.c:272
 proc_reg_read_iter+0x1c2/0x290 fs/proc/inode.c:299
 copy_splice_read+0x63a/0xb40 fs/splice.c:365
 do_splice_read fs/splice.c:985 [inline]
 splice_direct_to_actor+0x4af/0xc80 fs/splice.c:1089
 do_splice_direct_actor fs/splice.c:1207 [inline]
 do_splice_direct+0x289/0x3e0 fs/splice.c:1233
 do_sendfile+0x564/0x8a0 fs/read_write.c:1363
 __do_sys_sendfile64 fs/read_write.c:1424 [inline]
 __se_sys_sendfile64+0x17c/0x1e0 fs/read_write.c:1410
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Reported-by: syzbot+fcee6b76cf2e261c51a4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fcee6b76cf2e261c51a4
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: Add a warning
V2 -> V3: replace to WARN_ON_ONCE on a separate line

 block/genhd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/genhd.c b/block/genhd.c
index 9130e163e191..3a9c36ad6bbd 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -890,6 +890,9 @@ static int show_partition(struct seq_file *seqf, void *v)
 
 	rcu_read_lock();
 	xa_for_each(&sgp->part_tbl, idx, part) {
+		int partno = bdev_partno(part);
+
+		WARN_ON_ONCE(partno >= DISK_MAX_PARTS);
 		if (!bdev_nr_sectors(part))
 			continue;
 		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V3] block: no show partitions if partno corrupted
  2025-01-14 14:58         ` [PATCH V3] " Edward Adam Davis
@ 2025-01-14 15:02           ` Jens Axboe
  2025-01-14 15:15             ` Edward Adam Davis
  0 siblings, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2025-01-14 15:02 UTC (permalink / raw)
  To: Edward Adam Davis
  Cc: hare, linux-block, linux-kernel, syzbot+fcee6b76cf2e261c51a4,
	syzkaller-bugs

On 1/14/25 7:58 AM, Edward Adam Davis wrote:
> diff --git a/block/genhd.c b/block/genhd.c
> index 9130e163e191..3a9c36ad6bbd 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -890,6 +890,9 @@ static int show_partition(struct seq_file *seqf, void *v)
>  
>  	rcu_read_lock();
>  	xa_for_each(&sgp->part_tbl, idx, part) {
> +		int partno = bdev_partno(part);
> +
> +		WARN_ON_ONCE(partno >= DISK_MAX_PARTS);
>  		if (!bdev_nr_sectors(part))
>  			continue;
>  		seq_printf(seqf, "%4d  %7d %10llu %pg\n",

Surely you still want to continue for that condition?

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V3] block: no show partitions if partno corrupted
  2025-01-14 15:02           ` Jens Axboe
@ 2025-01-14 15:15             ` Edward Adam Davis
  2025-01-14 15:25               ` Jens Axboe
  0 siblings, 1 reply; 12+ messages in thread
From: Edward Adam Davis @ 2025-01-14 15:15 UTC (permalink / raw)
  To: axboe
  Cc: eadavis, hare, linux-block, linux-kernel,
	syzbot+fcee6b76cf2e261c51a4, syzkaller-bugs

On Tue, 14 Jan 2025 08:02:15 -0700, Jens Axboe wrote:
> > diff --git a/block/genhd.c b/block/genhd.c
> > index 9130e163e191..3a9c36ad6bbd 100644
> > --- a/block/genhd.c
> > +++ b/block/genhd.c
> > @@ -890,6 +890,9 @@ static int show_partition(struct seq_file *seqf, void *v)
> >
> >  	rcu_read_lock();
> >  	xa_for_each(&sgp->part_tbl, idx, part) {
> > +		int partno = bdev_partno(part);
> > +
> > +		WARN_ON_ONCE(partno >= DISK_MAX_PARTS);
> >  		if (!bdev_nr_sectors(part))
> >  			continue;
> >  		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
> 
> Surely you still want to continue for that condition?
No.
But like following, ok?
diff --git a/block/genhd.c b/block/genhd.c
index 9130e163e191..142b13620f0c 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -890,7 +890,10 @@ static int show_partition(struct seq_file *seqf, void *v)
 
        rcu_read_lock();
        xa_for_each(&sgp->part_tbl, idx, part) {
-               if (!bdev_nr_sectors(part))
+               int partno = bdev_partno(part);
+
+               WARN_ON_ONCE(partno >= DISK_MAX_PARTS);
+               if (!bdev_nr_sectors(part) || partno >= DISK_MAX_PARTS)
                        continue;
                seq_printf(seqf, "%4d  %7d %10llu %pg\n",
                           MAJOR(part->bd_dev), MINOR(part->bd_dev),


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V3] block: no show partitions if partno corrupted
  2025-01-14 15:15             ` Edward Adam Davis
@ 2025-01-14 15:25               ` Jens Axboe
  2025-01-14 15:34                 ` [PATCH V4] " Edward Adam Davis
  2025-01-14 16:21                 ` [PATCH V3] " Edward Adam Davis
  0 siblings, 2 replies; 12+ messages in thread
From: Jens Axboe @ 2025-01-14 15:25 UTC (permalink / raw)
  To: Edward Adam Davis
  Cc: hare, linux-block, linux-kernel, syzbot+fcee6b76cf2e261c51a4,
	syzkaller-bugs

On 1/14/25 8:15 AM, Edward Adam Davis wrote:
> On Tue, 14 Jan 2025 08:02:15 -0700, Jens Axboe wrote:
>>> diff --git a/block/genhd.c b/block/genhd.c
>>> index 9130e163e191..3a9c36ad6bbd 100644
>>> --- a/block/genhd.c
>>> +++ b/block/genhd.c
>>> @@ -890,6 +890,9 @@ static int show_partition(struct seq_file *seqf, void *v)
>>>
>>>  	rcu_read_lock();
>>>  	xa_for_each(&sgp->part_tbl, idx, part) {
>>> +		int partno = bdev_partno(part);
>>> +
>>> +		WARN_ON_ONCE(partno >= DISK_MAX_PARTS);
>>>  		if (!bdev_nr_sectors(part))
>>>  			continue;
>>>  		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
>>
>> Surely you still want to continue for that condition?
> No.

No?

> But like following, ok?
> diff --git a/block/genhd.c b/block/genhd.c
> index 9130e163e191..142b13620f0c 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -890,7 +890,10 @@ static int show_partition(struct seq_file *seqf, void *v)
>  
>         rcu_read_lock();
>         xa_for_each(&sgp->part_tbl, idx, part) {
> -               if (!bdev_nr_sectors(part))
> +               int partno = bdev_partno(part);
> +
> +               WARN_ON_ONCE(partno >= DISK_MAX_PARTS);
> +               if (!bdev_nr_sectors(part) || partno >= DISK_MAX_PARTS)
>                         continue;
>                 seq_printf(seqf, "%4d  %7d %10llu %pg\n",
>                            MAJOR(part->bd_dev), MINOR(part->bd_dev),

That's just silly...

	xa_for_each(&sgp->part_tbl, idx, part) {
		int partno = bdev_partno(part);

		if (!bdev_nr_sectors(part))
			continue;
		if (WARN_ON_ONCE(partno >= DISK_MAX_PARTS))
			continue;

		...
	}

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V4] block: no show partitions if partno corrupted
  2025-01-14 15:25               ` Jens Axboe
@ 2025-01-14 15:34                 ` Edward Adam Davis
  2025-01-14 16:21                 ` [PATCH V3] " Edward Adam Davis
  1 sibling, 0 replies; 12+ messages in thread
From: Edward Adam Davis @ 2025-01-14 15:34 UTC (permalink / raw)
  To: axboe
  Cc: eadavis, hare, linux-block, linux-kernel,
	syzbot+fcee6b76cf2e261c51a4, syzkaller-bugs

syzbot reported a global-out-of-bounds in number. [1]

Corrupted partno causes out-of-bounds access when accessing the hex_asc_upper
array.

To avoid this issue, skip partitions with partno greater than DISK_MAX_PARTS.

[1]
BUG: KASAN: global-out-of-bounds in number+0x3be/0xf40 lib/vsprintf.c:494
Read of size 1 at addr ffffffff8c5fc971 by task syz-executor351/5832

CPU: 0 UID: 0 PID: 5832 Comm: syz-executor351 Not tainted 6.13.0-rc6-next-20250107-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:378 [inline]
 print_report+0x169/0x550 mm/kasan/report.c:489
 kasan_report+0x143/0x180 mm/kasan/report.c:602
 number+0x3be/0xf40 lib/vsprintf.c:494
 pointer+0x764/0x1210 lib/vsprintf.c:2484
 vsnprintf+0x75a/0x1220 lib/vsprintf.c:2846
 seq_vprintf fs/seq_file.c:391 [inline]
 seq_printf+0x172/0x270 fs/seq_file.c:406
 show_partition+0x29f/0x3f0 block/genhd.c:905
 seq_read_iter+0x969/0xd70 fs/seq_file.c:272
 proc_reg_read_iter+0x1c2/0x290 fs/proc/inode.c:299
 copy_splice_read+0x63a/0xb40 fs/splice.c:365
 do_splice_read fs/splice.c:985 [inline]
 splice_direct_to_actor+0x4af/0xc80 fs/splice.c:1089
 do_splice_direct_actor fs/splice.c:1207 [inline]
 do_splice_direct+0x289/0x3e0 fs/splice.c:1233
 do_sendfile+0x564/0x8a0 fs/read_write.c:1363
 __do_sys_sendfile64 fs/read_write.c:1424 [inline]
 __se_sys_sendfile64+0x17c/0x1e0 fs/read_write.c:1410
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Reported-by: syzbot+fcee6b76cf2e261c51a4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fcee6b76cf2e261c51a4
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: Add a warning
V2 -> V3: replace to WARN_ON_ONCE on a separate line
V3 -> V4: add continue

 block/genhd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/genhd.c b/block/genhd.c
index 9130e163e191..a9a1d5a429aa 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -890,8 +890,12 @@ static int show_partition(struct seq_file *seqf, void *v)
 
 	rcu_read_lock();
 	xa_for_each(&sgp->part_tbl, idx, part) {
+		int partno = bdev_partno(part);
+
 		if (!bdev_nr_sectors(part))
 			continue;
+		if (WARN_ON_ONCE(partno >= DISK_MAX_PARTS))
+			continue;
 		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
 			   MAJOR(part->bd_dev), MINOR(part->bd_dev),
 			   bdev_nr_sectors(part) >> 1, part);
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V3] block: no show partitions if partno corrupted
  2025-01-14 15:25               ` Jens Axboe
  2025-01-14 15:34                 ` [PATCH V4] " Edward Adam Davis
@ 2025-01-14 16:21                 ` Edward Adam Davis
  1 sibling, 0 replies; 12+ messages in thread
From: Edward Adam Davis @ 2025-01-14 16:21 UTC (permalink / raw)
  To: axboe
  Cc: eadavis, hare, linux-block, linux-kernel,
	syzbot+fcee6b76cf2e261c51a4, syzkaller-bugs

On Tue, 14 Jan 2025 08:25:13 -0700, Jens Axboe wrote:
>> On Tue, 14 Jan 2025 08:02:15 -0700, Jens Axboe wrote:
>>>> diff --git a/block/genhd.c b/block/genhd.c
>>>> index 9130e163e191..3a9c36ad6bbd 100644
>>>> --- a/block/genhd.c
>>>> +++ b/block/genhd.c
>>>> @@ -890,6 +890,9 @@ static int show_partition(struct seq_file *seqf, void *v)
>>>>
>>>>  	rcu_read_lock();
>>>>  	xa_for_each(&sgp->part_tbl, idx, part) {
>>>> +		int partno = bdev_partno(part);
>>>> +
>>>> +		WARN_ON_ONCE(partno >= DISK_MAX_PARTS);
>>>>  		if (!bdev_nr_sectors(part))
>>>>  			continue;
>>>>  		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
>>>
>>> Surely you still want to continue for that condition?
>> No.
>
>No?
>
>> But like following, ok?
>> diff --git a/block/genhd.c b/block/genhd.c
>> index 9130e163e191..142b13620f0c 100644
>> --- a/block/genhd.c
>> +++ b/block/genhd.c
>> @@ -890,7 +890,10 @@ static int show_partition(struct seq_file *seqf, void *v)
>>
>>         rcu_read_lock();
>>         xa_for_each(&sgp->part_tbl, idx, part) {
>> -               if (!bdev_nr_sectors(part))
>> +               int partno = bdev_partno(part);
>> +
>> +               WARN_ON_ONCE(partno >= DISK_MAX_PARTS);
>> +               if (!bdev_nr_sectors(part) || partno >= DISK_MAX_PARTS)
>>                         continue;
>>                 seq_printf(seqf, "%4d  %7d %10llu %pg\n",
>>                            MAJOR(part->bd_dev), MINOR(part->bd_dev),
>
>That's just silly...
I checked WARN_ON_ONCE(), and when the condition is met, the subsequent
WARN_ON_ONCE() will still return true, so adding it will not affect the
judgment of the condition.
It just issues a warning the first time the condition is met, and it will
still return true if the condition is true.
>
>	xa_for_each(&sgp->part_tbl, idx, part) {
>		int partno = bdev_partno(part);
>
>		if (!bdev_nr_sectors(part))
>			continue;
>		if (WARN_ON_ONCE(partno >= DISK_MAX_PARTS))
>			continue;
>
>		...
>	}

Edward


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] block: no show partitions if partno corrupted
  2025-01-14 14:16       ` Jens Axboe
  2025-01-14 14:58         ` [PATCH V3] " Edward Adam Davis
@ 2025-01-15  6:46         ` Christoph Hellwig
  1 sibling, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2025-01-15  6:46 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Edward Adam Davis, hare, linux-block, linux-kernel,
	syzbot+fcee6b76cf2e261c51a4, syzkaller-bugs

On Tue, Jan 14, 2025 at 07:16:31AM -0700, Jens Axboe wrote:
> On 1/14/25 1:51 AM, Edward Adam Davis wrote:
> > diff --git a/block/genhd.c b/block/genhd.c
> > index 9130e163e191..8d539a4a3b37 100644
> > --- a/block/genhd.c
> > +++ b/block/genhd.c
> > @@ -890,7 +890,9 @@ static int show_partition(struct seq_file *seqf, void *v)
> >  
> >  	rcu_read_lock();
> >  	xa_for_each(&sgp->part_tbl, idx, part) {
> > -		if (!bdev_nr_sectors(part))
> > +		int partno = bdev_partno(part);
> > +
> > +		if (!bdev_nr_sectors(part) || WARN_ON(partno >= DISK_MAX_PARTS))
> >  			continue;
> >  		seq_printf(seqf, "%4d  %7d %10llu %pg\n",
> >  			   MAJOR(part->bd_dev), MINOR(part->bd_dev),
> 
> This should be a WARN_ON_ONCE(), and please put warn-on's on a separate
> line.

Ummm...

DISK_MAX_PARTS is 256.

bdev_partno reads form bdev->__bd_flags and masks out BD_PARTNO,
which is 255.

In other words we should never be able to get a value bigger than 255
from bdev_partno, so something is really fishy here that a WARN_ON in
the show function won't help with.

Also the fact that the low-level printf code trips over a 8-bit integer
sounds wrong, and if it does for something not caused by say a use
after free higher up we've got another deep problem there.

All of that has nothing to do with show_partition, though.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-01-15  6:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <67841058.050a0220.216c54.0034.GAE@google.com>
2025-01-14  2:28 ` [PATCH] block: no show partitions if partno corrupted Edward Adam Davis
2025-01-14  7:21   ` Hannes Reinecke
2025-01-14  8:51     ` [PATCH V2] " Edward Adam Davis
2025-01-14 14:16       ` Jens Axboe
2025-01-14 14:58         ` [PATCH V3] " Edward Adam Davis
2025-01-14 15:02           ` Jens Axboe
2025-01-14 15:15             ` Edward Adam Davis
2025-01-14 15:25               ` Jens Axboe
2025-01-14 15:34                 ` [PATCH V4] " Edward Adam Davis
2025-01-14 16:21                 ` [PATCH V3] " Edward Adam Davis
2025-01-15  6:46         ` [PATCH V2] " Christoph Hellwig
2025-01-14  5:29 ` [syzbot] [fs?] KASAN: global-out-of-bounds Read in number syzbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox