linux-kernel - Re: [PATCH 11/27] bcache: io.c: use bio_set_vec

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:	Tue, 5 Apr 2016 16:35:07 -0800
From:	Kent Overstreet <kent.overstreet@...il.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Ming Lei <tom.leiming@...il.com>, Jens Axboe <axboe@...com>,
	linux-kernel@...r.kernel.org, linux-block@...r.kernel.org,
	Boaz Harrosh <boaz@...xistor.com>,
	Shaohua Li <shli@...nel.org>,
	"open list:BCACHE (BLOCK LAYER CACHE)" <linux-bcache@...r.kernel.org>,
	"open list:SOFTWARE RAID (Multiple Disks) SUPPORT" 
	<linux-raid@...r.kernel.org>
Subject: Re: [PATCH 11/27] bcache: io.c: use bio_set_vec_table

On Tue, Apr 05, 2016 at 05:49:02AM -0700, Christoph Hellwig wrote:
> On Tue, Apr 05, 2016 at 07:56:56PM +0800, Ming Lei wrote:
> > diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
> > index 86a0bb8..1c48462 100644
> > --- a/drivers/md/bcache/io.c
> > +++ b/drivers/md/bcache/io.c
> > @@ -26,8 +26,7 @@ struct bio *bch_bbio_alloc(struct cache_set *c)
> >  
> >  	bio_init(bio);
> >  	bio->bi_flags		|= BIO_POOL_NONE << BIO_POOL_OFFSET;
> > -	bio->bi_max_vecs	 = bucket_pages(c);
> > -	bio->bi_io_vec		 = bio->bi_inline_vecs;
> > +	bio_set_vec_table(bio, bio->bi_inline_vecs, bucket_pages(c));
> 
> All this bcache code needs to move away from bio_init on a bio
> embedded in a driver private structure toward properly using
> bio_alloc / bio_alloc_bioset.  That will also fix the crash
> with bcache over md that Shaohua reported, so I'd suggest to fast
> track this part of the series.

Why?

bio_init() is a publicly exported function, it's always been one and bcache is
ot the only driver to use it directly.

bios with > BIO_MAX_PAGES bvecs is a separate issue; I would argue that the bug
is in md's queue_limits; it uses blk_set_stacking_limits() which sets
max_segments = USHRT_MAX, which is wrong if it's going to clone the biovec.