Hi Ming,
(sorry, re-sending in plain text; previous reply had HTML by mistake,
and bounced in linux-block.)
On Mon, Jun 1, 2020 at 4:34 AM Ming Lei <ming.lei@xxxxxxxxxx> wrote:
>
> On Sun, May 31, 2020 at 09:55:20PM -0300, Mauricio Faria de Oliveira wrote:
> > It's possible for a block driver to set logical block size to
> > a value greater than page size incorrectly; e.g. bcache takes
> > the value from the superblock, set by the user w/ make-bcache.
> >
> > This causes a BUG/NULL pointer dereference in the path:
> >
> > __blkdev_get()
> > -> set_init_blocksize() // set i_blkbits based on ...
> > -> bdev_logical_block_size()
> > -> queue_logical_block_size() // ... this value
> > -> bdev_disk_changed()
> > ...
> > -> blkdev_readpage()
> > -> block_read_full_page()
> > -> create_page_buffers() // size = 1 << i_blkbits
> > -> create_empty_buffers() // give size/take pointer
> > -> alloc_page_buffers() // return NULL
> > .. BUG!
> >
> > Because alloc_page_buffers() is called with size > PAGE_SIZE,
> > thus it initializes head = NULL, skips the loop, return head;
> > then create_empty_buffers() gets (and uses) the NULL pointer.
> >
> > This has been around longer than commit ad6bf88a6c19 ("block:
> > fix an integer overflow in logical block size"); however, it
> > increased the range of values that can trigger the issue.
> >
> > Previously only 8k/16k/32k (on x86/4k page size) would do it,
> > as greater values overflow unsigned short to zero, and queue_
> > logical_block_size() would then use the default of 512.
> >
> > Now the range with unsigned int is much larger, and one user
> > with an (incorrect) 512k value, which happened to be zero'ed
> > previously and work fine, hits the issue -- the zero is gone,
> > and queue_logical_block_size() does return 512k (> PAGE_SIZE)
>
> There is only very limited such potential users(loop, virtio-blk,
> xen-blkfront), so could you fix the user instead of working around
> queue_logical_block_size()?
>
Thanks for reviewing.
I can take a look at that, sure, but think the current approach may
still be useful? as it prevents the current, and future potential
users too.
Cheers,
> thanks,
> Ming
>
--
Mauricio Faria de Oliveira
