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,
Ming
