Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> On 7/16/26 16:18, Greg Kroah-Hartman wrote:
> > On Thu, Jul 16, 2026 at 10:55:42AM +0200, David Hildenbrand (Arm) wrote:
> >> On 7/15/26 18:41, Hari Mishal wrote:
> >>> The device_block_size read from the virtio-mem config space is used as a
> >>> divisor and also in ALIGN_DOWN() further down the code path in the
> >>> driver without further validation. A zero value leads to a division by
> >>> zero, and a non-power-of-two value corrupts the ALIGN_DOWN() bitmask
> >>> arithmetic leading to a misreporting of guest-usable ram, post crash.
> >>>
> >>> Reject both at init time instead of trusting the device.
> >>>
> >>> Signed-off-by: Hari Mishal <harimishal1@gmail.com>
> >>> ---
> >>> v2: dropped the redundant explicit zero check, since
> >>>     is_power_of_2(0) already returns false.
> >>>
> >>>  drivers/virtio/virtio_mem.c | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> >>> index 11c441501582..0e04fec458af 100644
> >>> --- a/drivers/virtio/virtio_mem.c
> >>> +++ b/drivers/virtio/virtio_mem.c
> >>> @@ -2847,6 +2847,12 @@ static int virtio_mem_init(struct virtio_mem *vm)
> >>>  			&vm->plugged_size);
> >>>  	virtio_cread_le(vm->vdev, struct virtio_mem_config, block_size,
> >>>  			&vm->device_block_size);
> >>> +	if (!is_power_of_2(vm->device_block_size)) {
> >>> +		dev_err(&vm->vdev->dev,
> >>> +			"invalid device block size: 0x%llx\n",
> >>> +			(unsigned long long)vm->device_block_size);
> >>> +		return -EINVAL;
> >>> +	}
> >>
> >> The spec states "The device MUST set block_size to a power of two."
> >>
> >> I'm missing the point here.
> > 
> > So what happens if we have a non-spec-compliant device?  Shouldn't we be
> > attempting to verify this before doing something with the data?
> 
> The problem I see is that there are plenty of other devices where a
> non-compliant device might cause problems.
> 
> Like, we request to hotplug some memory block and our device ACKs it, but it
> simply didn't do that.
> 
> Or we request to hotunplug a memory block and instead it hotplugs some random
> other memory block.
> 
> Or we sense whether a memory block is plugged and the device lies to us.
> 
> > 
> > Or do we just always trust virtio mem devices explicitly?
> 
> It's hard for me to understand where we draw the line, really.
> 
> But maybe MST can clarify what we care about in virtio world where the
> hypervisor is fully in charge of the device,

That would be good to figure out, and write down somewhere, so we know
what to worry about here.

Right now, for almost all subsystems, Linux trusts the hardware
explicitly.  But for some subsystems, it doesn't, or doesn't until
"something" happens (example, USB devices are untrusted until we bind a
driver to the device, and then they are trusted.)

But there are some people that want to never trust the hardware, or have
it in different states, specifically for virtual machines like this
driver operates in.  The CoC model is just that, and patches from Dan
are working toward clearing this up a lot for some types of devices (PCI
and IOMMU), but I don't think they have thought about virtio devices
yet...

thanks,

greg k-h