Re: [PATCH v1 0/2] virtio: Add length checks for device writable portions

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

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.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]