On Thu, Feb 27, 2025 at 04:17:47PM +0800, Stefan Hajnoczi wrote: > On Tue, Feb 25, 2025 at 01:31:04AM +0200, Max Gurtovoy wrote: > > Hi, > > > > This patch series introduces safety checks in virtio-blk and virtio-fs > > drivers to ensure proper handling of device-writable buffer lengths as > > specified by the virtio specification. > > > > The virtio specification states: > > "The driver MUST NOT make assumptions about data in device-writable > > buffers beyond the first len bytes, and SHOULD ignore this data." > > > > To align with this requirement, we introduce checks in both drivers to > > verify that the length of data written by the device is at least as > > large as the expected/needed payload. > > > > If this condition is not met, we set an I/O error status to prevent > > processing of potentially invalid or incomplete data. > > > > These changes improve the robustness of the drivers and ensure better > > compliance with the virtio specification. > > > > Max Gurtovoy (2): > > virtio_blk: add length check for device writable portion > > virtio_fs: add length check for device writable portion > > > > drivers/block/virtio_blk.c | 20 ++++++++++++++++++++ > > fs/fuse/virtio_fs.c | 9 +++++++++ > > 2 files changed, 29 insertions(+) > > > > -- > > 2.18.1 > > > > There are 3 cases: > 1. The device reports len correctly. > 2. The device reports len incorrectly, but the in buffers contain valid > data. > 3. The device reports len incorrectly and the in buffers contain invalid > data. > > Case 1 does not change behavior. > > Case 3 never worked in the first place. This patch might produce an > error now where garbage was returned in the past. > > It's case 2 that I'm worried about: users won't be happy if the driver > stops working with a device that previously worked.
Interestingly, when virtio core unmaps buffers, it always syncronizes the whole range. virtio net jumps through hoops to only sync a part. So Max, my suggestion is to maybe try a combination of dma sync + unmap, and then this becomes an optimization. It might be worth it, for when using swiotlb - but the gain will have to be measured. > Should we really risk breakage for little benefit? > > I remember there were cases of invalid len values reported by devices in > the past. Michael might have thoughts about this. > > Stefan
