linux-kernel - Re: [PATCH V2] block: no show partitions if partno corrupted

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] [day] [month] [year] [list]

Message-ID: <Z4dZvH5WuOeNTgXg@infradead.org>
Date: Tue, 14 Jan 2025 22:46:20 -0800
From: Christoph Hellwig <hch@...radead.org>
To: Jens Axboe <axboe@...nel.dk>
Cc: Edward Adam Davis <eadavis@...com>, hare@...e.de,
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
	syzbot+fcee6b76cf2e261c51a4@...kaller.appspotmail.com,
	syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH V2] block: no show partitions if partno corrupted

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.