On Tue, Oct 5, 2021 at 6:42 PM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: > > On Mon, Oct 04, 2021 at 11:27:29AM -0400, Michael S. Tsirkin wrote: > > On Mon, Aug 09, 2021 at 06:16:09PM +0800, Xie Yongji wrote: > > > An untrusted device might presents an invalid block size > > > in configuration space. This tries to add validation for it > > > in the validate callback and clear the VIRTIO_BLK_F_BLK_SIZE > > > feature bit if the value is out of the supported range. > > > > > > And we also double check the value in virtblk_probe() in > > > case that it's changed after the validation. > > > > > > Signed-off-by: Xie Yongji <xieyongji@xxxxxxxxxxxxx> > > > > So I had to revert this due basically bugs in QEMU. > > > > My suggestion at this point is to try and update > > blk_queue_logical_block_size to BUG_ON when the size > > is out of a reasonable range. > > > > This has the advantage of fixing more hardware, not just virtio. > > > > Stefan also pointed out this duplicates the logic from > > if (blksize < 512 || blksize > PAGE_SIZE || !is_power_of_2(blksize)) > return -EINVAL; > > > and a bunch of other places. > > > Would it be acceptable for blk layer to validate the input > instead of having each driver do it's own thing? > Maybe inside blk_queue_logical_block_size? > Now the nbd and loop driver will validate the blksize and return error if it's invalid. But the nvme and null_blk driver just corrects the blksize to a reasonable range without returning any error. I'm not sure which way the block layer should follow. Or just simplify the logic in nbd and loop driver? Thanks, Yongji
