[PATCH 0/4] virtio: validate device-reported values across drivers

[PATCH 0/4] virtio: validate device-reported values across drivers

[Hari Mishal]

These four patches harden virtio-mem, virtio_input, and virtio_console 
against untrusted values coming from the device/backend side of the virtio
interface. The fourth fixes a use-after-free in virtio_console caused by a
missing refcount.

Hari Mishal (4):
  virtio-mem: validate device-reported block size
  virtio_input: validate device-reported multitouch slot count
  virtio_console: avoid NULL portdev dereference in in_intr()
  virtio_console: take a kref in find_port_by_vq() to fix port UAF

 drivers/char/virtio_console.c | 18 +++++++++++++++++-
 drivers/virtio/virtio_input.c |  4 ++++
 drivers/virtio/virtio_mem.c   |  7 +++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

-- 
2.43.0

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

[Hari Mishal]

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 guest ram, post
crash. Reject both at init time instead of trusting the device.

Assisted-by: gkh_clanker:t1000
Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
 drivers/virtio/virtio_mem.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 11c441501582..43d12ec7c323 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -2847,6 +2847,13 @@ 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 (!vm->device_block_size ||
+	    !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;
+	}
 	virtio_cread_le(vm->vdev, struct virtio_mem_config, node_id,
 			&node_id);
 	vm->nid = virtio_mem_translate_node_id(vm, node_id);
-- 
2.43.0

[PATCH 2/4] virtio_input: validate device-reported multitouch slot count

[Hari Mishal]

nslots is derived from the ABS_MT_SLOT maximum reported by the
virtio device. A device could report a bogus maximum (e.g. -1)
making nslots = 0, which input_mt_init_slots() does not reject;
it returns success without allocating any slot storage, silently
leaving the device registered as multitouch capable with no
backing state. Reject non-positive slot counts before calling
input_mt_init_slots().

Assisted-by: gkh_clanker:t1000
Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
 drivers/virtio/virtio_input.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index deec24e8e682..2cc19782cdd3 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -312,6 +312,10 @@ static int virtinput_probe(struct virtio_device *vdev)
 
 		if (test_bit(ABS_MT_SLOT, vi->idev->absbit)) {
 			nslots = input_abs_get_max(vi->idev, ABS_MT_SLOT) + 1;
+			if (nslots <= 0) {
+				err = -EINVAL;
+				goto err_mt_init_slots;
+			}
 			err = input_mt_init_slots(vi->idev, nslots, 0);
 			if (err)
 				goto err_mt_init_slots;
-- 
2.43.0

[PATCH 3/4] virtio_console: avoid NULL portdev dereference in in_intr()

[Hari Mishal]

A port's virtqueue is not torn down immediately if the port itself is
hot-unplugged (unplug_port() only nulls port->portdev; the vq
callback stays registered until the whole device is removed).
If in_intr() fires for a port in that window it dereferences
port->portdev->vdev via is_rproc_serial(), crashing on the NULL
portdev.

Bail out early when portdev has already been cleared.

Assisted-by: gkh_clanker:t1000
Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
 drivers/char/virtio_console.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 198b97314168..faef362dae85 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1720,6 +1720,11 @@ static void in_intr(struct virtqueue *vq)
 	}
 
 	spin_lock_irqsave(&port->inbuf_lock, flags);
+	if (!port->portdev) {
+		/* Port is being unplugged, ignore further data. */
+		spin_unlock_irqrestore(&port->inbuf_lock, flags);
+		return;
+	}
 	port->inbuf = get_inbuf(port);
 
 	/*
-- 
2.43.0

[PATCH 4/4] virtio_console: take a kref in find_port_by_vq() to fix port UAF

[Hari Mishal]

find_port_by_vq() returns a raw struct port pointer without taking a
reference on it, unlike find_port_by_devt_in_portdev() which does.
find_port_by_vq()'s only two callers, in_intr() and out_intr(), run
as virtqueue interrupt callbacks, entirely independent of and
possibly concurrently with unplug_port(), which itself runs from a
workqueue when the host sends a VIRTIO_CONSOLE_PORT_REMOVE control
message.

unplug_port() removes the port from portdev->ports under ports_lock,
then later drops its last reference with kref_put(), freeing it via
remove_port(). find_port_by_vq() also walks portdev->ports under
ports_lock, so if it finds the port still on the list, the list
removal, and therefore the eventual kref_put(), has not happened yet,
and taking a reference at that point is always safe. Without doing
so, in_intr()/out_intr() can be left holding a pointer to a port that
unplug_port() frees on another core before they are done using it.

Both triggers are host-controlled as the host decides when to send the
PORT_REMOVE control message and when to kick the port's data vq. So a
malicious backend could race the two on purpose, without any guest side
cooperation. The freed object is a generic kmalloc allocation containing
a wait_queue_head_t, which in_intr()/out_intr() pass to
wake_up_interruptible() after touching the stale pointer.
wake_up_interruptible() invokes a function pointer read out of the
wait queue's entries. If the freed slab slot is reclaimed with
attacker influenced content before that call, then this is an arbitrary
function call primitive rather than just undefined behaviour.

Take a reference in find_port_by_vq() while still holding ports_lock,
matching find_port_by_devt_in_portdev(), and release it in in_intr()
and out_intr() once they are done with the port.

Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
 drivers/char/virtio_console.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index faef362dae85..1b7593684ed9 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -304,6 +304,11 @@ static struct port *find_port_by_id(struct ports_device *portdev, u32 id)
 	return port;
 }
 
+/*
+ * The port object's reference is incremented now and
+ * it is the caller's responsibility to decrement it
+ */
+
 static struct port *find_port_by_vq(struct ports_device *portdev,
 				    struct virtqueue *vq)
 {
@@ -312,8 +317,10 @@ static struct port *find_port_by_vq(struct ports_device *portdev,
 
 	spin_lock_irqsave(&portdev->ports_lock, flags);
 	list_for_each_entry(port, &portdev->ports, list)
-		if (port->in_vq == vq || port->out_vq == vq)
+		if (port->in_vq == vq || port->out_vq == vq) {
+			kref_get(&port->kref);
 			goto out;
+		}
 	port = NULL;
 out:
 	spin_unlock_irqrestore(&portdev->ports_lock, flags);
@@ -1706,6 +1713,7 @@ static void out_intr(struct virtqueue *vq)
 	}
 
 	wake_up_interruptible(&port->waitqueue);
+	kref_put(&port->kref, remove_port);
 }
 
 static void in_intr(struct virtqueue *vq)
@@ -1723,6 +1731,7 @@ static void in_intr(struct virtqueue *vq)
 	if (!port->portdev) {
 		/* Port is being unplugged, ignore further data. */
 		spin_unlock_irqrestore(&port->inbuf_lock, flags);
+		kref_put(&port->kref, remove_port);
 		return;
 	}
 	port->inbuf = get_inbuf(port);
@@ -1756,6 +1765,8 @@ static void in_intr(struct virtqueue *vq)
 
 	if (is_console_port(port) && hvc_poll(port->cons.hvc))
 		hvc_kick();
+
+	kref_put(&port->kref, remove_port);
 }
 
 static void control_intr(struct virtqueue *vq)
-- 
2.43.0

[PATCH v2 4/4] virtio_console: take a kref in find_port_by_vq() to fix port UAF

[Hari Mishal]

find_port_by_vq() returns a raw struct port pointer without taking a
reference on it, unlike find_port_by_devt_in_portdev() which does.
find_port_by_vq()'s only two callers, in_intr() and out_intr(), run as
virtqueue interrupt callbacks, entirely independent of and possibly
concurrently with unplug_port(), which itself runs from a workqueue when
the host sends a VIRTIO_CONSOLE_PORT_REMOVE control message.

unplug_port() removes the port from portdev->ports under ports_lock,
then later drops its last reference with kref_put(), freeing it via
remove_port(). find_port_by_vq() also walks portdev->ports under
ports_lock, so if it finds the port still on the list, the list removal,
and therefore the eventual kref_put(), has not happened yet, and taking
a reference at that point is always safe. Without doing so,
in_intr()/out_intr() can be left holding a pointer to a port that
unplug_port() frees on another core before they are done using it.

Both triggers are host-controlled as the host decides when to send the
PORT_REMOVE control message and when to kick the port's data vq. So a
malicious backend could race the two on purpose, without any guest side
cooperation. The freed object is a generic kmalloc allocation containing
a wait_queue_head_t, which in_intr()/out_intr() pass to
wake_up_interruptible() after touching the stale pointer.
wake_up_interruptible() invokes a function pointer read out of the wait
queue's entries. If the freed slab slot is reclaimed with attacker
influenced content before that call, then this is an arbitrary function
call primitive rather than just undefined behaviour.

Take a reference in find_port_by_vq() while still holding ports_lock,
matching find_port_by_devt_in_portdev(), and release it in in_intr() and
out_intr() once they are done with the port.

Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
v2: reworded the comment above find_port_by_vq() for clarity, no
    functional change since v1.

 drivers/char/virtio_console.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index faef362dae85..1b63afe24e29 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -304,6 +304,12 @@ static struct port *find_port_by_id(struct ports_device *portdev, u32 id)
 	return port;
 }
 
+/*
+ * Finds a port by the virtqueue and returns a pointer to struct port
+ * with the reference count incremented.
+ *
+ * Callers MUST decrement it when finished.
+ */
 static struct port *find_port_by_vq(struct ports_device *portdev,
 				    struct virtqueue *vq)
 {
@@ -312,8 +318,10 @@ static struct port *find_port_by_vq(struct ports_device *portdev,
 
 	spin_lock_irqsave(&portdev->ports_lock, flags);
 	list_for_each_entry(port, &portdev->ports, list)
-		if (port->in_vq == vq || port->out_vq == vq)
+		if (port->in_vq == vq || port->out_vq == vq) {
+			kref_get(&port->kref);
 			goto out;
+		}
 	port = NULL;
 out:
 	spin_unlock_irqrestore(&portdev->ports_lock, flags);
@@ -1706,6 +1714,7 @@ static void out_intr(struct virtqueue *vq)
 	}
 
 	wake_up_interruptible(&port->waitqueue);
+	kref_put(&port->kref, remove_port);
 }
 
 static void in_intr(struct virtqueue *vq)
@@ -1723,6 +1732,7 @@ static void in_intr(struct virtqueue *vq)
 	if (!port->portdev) {
 		/* Port is being unplugged, ignore further data. */
 		spin_unlock_irqrestore(&port->inbuf_lock, flags);
+		kref_put(&port->kref, remove_port);
 		return;
 	}
 	port->inbuf = get_inbuf(port);
@@ -1756,6 +1766,8 @@ static void in_intr(struct virtqueue *vq)
 
 	if (is_console_port(port) && hvc_poll(port->cons.hvc))
 		hvc_kick();
+
+	kref_put(&port->kref, remove_port);
 }
 
 static void control_intr(struct virtqueue *vq)
-- 
2.43.0

Re: [PATCH 2/4] virtio_input: validate device-reported multitouch slot count

[Michael S. Tsirkin]

On Wed, Jul 15, 2026 at 04:22:41PM +0200, Hari Mishal wrote:
> nslots is derived from the ABS_MT_SLOT maximum reported by the
> virtio device. A device could report a bogus maximum (e.g. -1)
> making nslots = 0, which input_mt_init_slots() does not reject;
> it returns success without allocating any slot storage, silently
> leaving the device registered as multitouch capable with no
> backing state.

So let's disable multitouch instead?

> Reject non-positive slot counts before calling
> input_mt_init_slots().
> 
> Assisted-by: gkh_clanker:t1000
> Signed-off-by: Hari Mishal <harimishal1@gmail.com>
> ---
>  drivers/virtio/virtio_input.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
> index deec24e8e682..2cc19782cdd3 100644
> --- a/drivers/virtio/virtio_input.c
> +++ b/drivers/virtio/virtio_input.c
> @@ -312,6 +312,10 @@ static int virtinput_probe(struct virtio_device *vdev)
>  
>  		if (test_bit(ABS_MT_SLOT, vi->idev->absbit)) {
>  			nslots = input_abs_get_max(vi->idev, ABS_MT_SLOT) + 1;
> +			if (nslots <= 0) {
> +				err = -EINVAL;
> +				goto err_mt_init_slots;
> +			}
>  			err = input_mt_init_slots(vi->idev, nslots, 0);
>  			if (err)
>  				goto err_mt_init_slots;
> -- 
> 2.43.0

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

[Michael S. Tsirkin]

On Wed, Jul 15, 2026 at 04:22:40PM +0200, 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 guest ram, post
> crash. Reject both at init time instead of trusting the device.
> 
> Assisted-by: gkh_clanker:t1000
> Signed-off-by: Hari Mishal <harimishal1@gmail.com>
> ---
>  drivers/virtio/virtio_mem.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 11c441501582..43d12ec7c323 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -2847,6 +2847,13 @@ 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 (!vm->device_block_size ||
> +	    !is_power_of_2(vm->device_block_size)) {

0 is not a power of 2, why do we need to check twice?

> +		dev_err(&vm->vdev->dev,
> +			"invalid device block size: 0x%llx\n",
> +			(unsigned long long)vm->device_block_size);
> +		return -EINVAL;
> +	}
>  	virtio_cread_le(vm->vdev, struct virtio_mem_config, node_id,
>  			&node_id);
>  	vm->nid = virtio_mem_translate_node_id(vm, node_id);
> -- 
> 2.43.0

Re: [PATCH 2/4] virtio_input: validate device-reported multitouch slot count

[Hari Mishal]

On Wed, Jul 15, 2026 at 5:50 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jul 15, 2026 at 04:22:41PM +0200, Hari Mishal wrote:
> > nslots is derived from the ABS_MT_SLOT maximum reported by the
> > virtio device. A device could report a bogus maximum (e.g. -1)
> > making nslots = 0, which input_mt_init_slots() does not reject;
> > it returns success without allocating any slot storage, silently
> > leaving the device registered as multitouch capable with no
> > backing state.
>
> So let's disable multitouch instead?
>

So rather than failing the whole probe, just warn and clear
ABS_MT_SLOT from absbit in that case, so the rest of the device
still registers? I took my lead from input_mt_init_slots(), which
has its own internal cap and returns -EINVAL when the device
reports more than 1024 slots.
Shall I modify that case to get the same "warn and disable
multitouch" for consistency, or is a hard failure better there since
it's a different type of bad device data?
Happy to fix it either way! Just want to confirm before sending V2.

Cheers,
Hari

Re: [PATCH 2/4] virtio_input: validate device-reported multitouch slot count

[Michael S. Tsirkin]

On Wed, Jul 15, 2026 at 06:07:56PM +0200, Hari Mishal wrote:
> On Wed, Jul 15, 2026 at 5:50 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Jul 15, 2026 at 04:22:41PM +0200, Hari Mishal wrote:
> > > nslots is derived from the ABS_MT_SLOT maximum reported by the
> > > virtio device. A device could report a bogus maximum (e.g. -1)
> > > making nslots = 0, which input_mt_init_slots() does not reject;
> > > it returns success without allocating any slot storage, silently
> > > leaving the device registered as multitouch capable with no
> > > backing state.
> >
> > So let's disable multitouch instead?
> >
> 
> So rather than failing the whole probe, just warn and clear
> ABS_MT_SLOT from absbit in that case, so the rest of the device
> still registers? I took my lead from input_mt_init_slots(), which
> has its own internal cap and returns -EINVAL when the device
> reports more than 1024 slots.
> Shall I modify that case to get the same "warn and disable
> multitouch" for consistency, or is a hard failure better there since
> it's a different type of bad device data?

I don't really know enough for sure but generally if the device can
kinda work it's better than not working, and adding
a capability to a device that driver can't use should
generally just ignore the capability, not fail probe.

> Happy to fix it either way! Just want to confirm before sending V2.
> 
> Cheers,
> Hari

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

[Hari Mishal]

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;
+	}
 	virtio_cread_le(vm->vdev, struct virtio_mem_config, node_id,
 			&node_id);
 	vm->nid = virtio_mem_translate_node_id(vm, node_id);
-- 
2.43.0

[PATCH v2 2/4] virtio_input: validate device-reported multitouch slot count

[Hari Mishal]

nslots is derived from the ABS_MT_SLOT maximum reported by the virtio
device. A device could report a bogus maximum (e.g. -1) making nslots <=
0, and input_mt_init_slots() can independently fail too (e.g. it rejects
slot counts over 1024). Neither case was handled here, and
input_mt_init_slots() failing took down the whole probe, losing the rest
of the input device over a problem isolated to multitouch.

Instead of failing the probe, warn and disable the ABS_MT_SLOT
capability in either case, and let the rest of the device register
normally. This recovery assumes input_mt_init_slots() is called with
flags == 0: with nonzero flags it can fail after already mutating the
input_dev (e.g. via input_set_abs_params()), which this recovery does
not account for.

Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
v2: per review, disable multitouch instead of failing the whole
    probe when the reported slot count is invalid or
    input_mt_init_slots() otherwise fails.

 drivers/virtio/virtio_input.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index deec24e8e682..786f5150724f 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -312,9 +312,25 @@ static int virtinput_probe(struct virtio_device *vdev)
 
 		if (test_bit(ABS_MT_SLOT, vi->idev->absbit)) {
 			nslots = input_abs_get_max(vi->idev, ABS_MT_SLOT) + 1;
-			err = input_mt_init_slots(vi->idev, nslots, 0);
-			if (err)
-				goto err_mt_init_slots;
+			if (nslots <= 0) {
+				dev_warn(&vdev->dev,
+					 "invalid multitouch slot count %d, disabling multitouch\n",
+					 nslots);
+				__clear_bit(ABS_MT_SLOT, vi->idev->absbit);
+			} else {
+				/*
+				 * flags is 0: input_mt_init_slots() can only fail
+				 * before touching *idev, so the recovery below is
+				 * a full rollback.
+				 */
+				err = input_mt_init_slots(vi->idev, nslots, 0);
+				if (err) {
+					dev_warn(&vdev->dev,
+						 "failed to init %d multitouch slots (%d), disabling multitouch\n",
+						 nslots, err);
+					__clear_bit(ABS_MT_SLOT, vi->idev->absbit);
+				}
+			}
 		}
 	}
 
@@ -331,7 +347,6 @@ static int virtinput_probe(struct virtio_device *vdev)
 	spin_lock_irqsave(&vi->lock, flags);
 	vi->ready = false;
 	spin_unlock_irqrestore(&vi->lock, flags);
-err_mt_init_slots:
 	input_free_device(vi->idev);
 err_input_alloc:
 	vdev->config->del_vqs(vdev);
-- 
2.43.0

Re: [PATCH 2/4] virtio_input: validate device-reported multitouch slot count

[Dmitry Torokhov]

On Wed, Jul 15, 2026 at 12:11:12PM -0400, Michael S. Tsirkin wrote:
> On Wed, Jul 15, 2026 at 06:07:56PM +0200, Hari Mishal wrote:
> > On Wed, Jul 15, 2026 at 5:50 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Wed, Jul 15, 2026 at 04:22:41PM +0200, Hari Mishal wrote:
> > > > nslots is derived from the ABS_MT_SLOT maximum reported by the
> > > > virtio device. A device could report a bogus maximum (e.g. -1)
> > > > making nslots = 0, which input_mt_init_slots() does not reject;
> > > > it returns success without allocating any slot storage, silently
> > > > leaving the device registered as multitouch capable with no
> > > > backing state.
> > >
> > > So let's disable multitouch instead?
> > >
> > 
> > So rather than failing the whole probe, just warn and clear
> > ABS_MT_SLOT from absbit in that case, so the rest of the device
> > still registers? I took my lead from input_mt_init_slots(), which
> > has its own internal cap and returns -EINVAL when the device
> > reports more than 1024 slots.
> > Shall I modify that case to get the same "warn and disable
> > multitouch" for consistency, or is a hard failure better there since
> > it's a different type of bad device data?
> 
> I don't really know enough for sure but generally if the device can
> kinda work it's better than not working, and adding
> a capability to a device that driver can't use should
> generally just ignore the capability, not fail probe.

What is the failure mode if we keep the ABS_MT_SLOT capability? Does the
kernel crash? And if this can cause crash then we should fix
input_mt_init_slots() to reject requests for 0 slots with -EINVAL.

Thanks.

-- 
Dmitry

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

[David Hildenbrand (Arm)]

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.

-- 
Cheers,

David

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

[Greg Kroah-Hartman]

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?

Or do we just always trust virtio mem devices explicitly?

thanks,
greg k-h

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

[David Hildenbrand (Arm)]

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,

-- 
Cheers,

David

Re: [PATCH 2/4] virtio_input: validate device-reported multitouch slot count

[Dmitry Torokhov]

[ Just realized that CC was dropped in the email I was replying to, so
  restoring and resending... ]

On Thu, Jul 16, 2026 at 10:28:36AM -0700, Dmitry Torokhov wrote:
> On Thu, Jul 16, 2026 at 12:34:57PM +0200, Hari Mishal wrote:
> > > What is the failure mode if we keep the ABS_MT_SLOT capability? Does the
> > > kernel crash? And if this can cause crash then we should fix
> > > input_mt_init_slots() to reject requests for 0 slots with -EINVAL.
> > >
> > 
> > No, it doesn't crash. I think every place in the input core that touches
> > dev->mt guards against it being NULL: input_handle_abs_event() and
> > the mt_slots check in input.c, and evdev's EVIOCGMTSLOTS ioctl
> > handler all explicitly check for NULL and degrade cleanly instead of
> > dereferencing. From my understanding, the worst case is what the
> > original commit message already covers: the device advertises
> > multitouch support it can't back.
> 
> So what? I still do not see the problem. Let's say I have a device that
> properly supports multitouch and has slots, but then never sends any
> events because firmware is buggy. How would that affect anything?
> 
> If there is no crash that I would leave the driver alone.
> 
> And we need to remember that we are dealing with a hypervisor here that
> normally had higher level of trust than the VM. If it messes up we do
> not have to clean up after it. This scenario is different from user
> attaching a malicious USB device to their system and getting owned.
> 
> Thanks.
> 

-- 
Dmitry

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

[Greg Kroah-Hartman]

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

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

[Michael S. Tsirkin]

On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > 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,

Generally:
- The guest is expected to whitelist drivers (most drivers have not
  been audited).
- Denial of service by the hypervisor is not considered a threat
  (this is how cloud vendors charge their clients, by denying
  service to whoever did not pay).



-- 
MST

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

[David Hildenbrand (Arm)]

On 7/17/26 07:48, Michael S. Tsirkin wrote:
> On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
>>> 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,
> 
> Generally:
> - The guest is expected to whitelist drivers (most drivers have not
>   been audited).

But even if you audited your driver, who makes sure that we consider all ways
where the device could mess with us?

Something feels off here.

Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
to be corrected.

-- 
Cheers,

David

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

[Michael S. Tsirkin]

On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> >>> 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,
> > 
> > Generally:
> > - The guest is expected to whitelist drivers (most drivers have not
> >   been audited).
> 
> But even if you audited your driver, who makes sure that we consider all ways
> where the device could mess with us?

A lot of this is up to a correct setup. For example, make sure all
filesystems are encrypted and refuse to mount unencrypted ones.

> Something feels off here.
> 
> Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> to be corrected.

Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
	It is important to note
	that this doesn’t imply that the host or VMM are intentionally
	malicious, but that there exists a security value in having a small CoCo
	VM TCB.

and

	While traditionally the host has unlimited access to guest data and can
	leverage this access to attack the guest, the CoCo systems mitigate such
	attacks by adding security features like guest data confidentiality and
	integrity protection.


now, when we are talking about "mitigation" it is indeed becoming a bit
murky.


For me, a rule of thumb I came up with is that if the validation happens
to also be helful for users e.g. to work around buggy devices,
or maybe because we feel failing gracefully is nice because this
will allow to later make use of this config and old drivers will
fail but at least not panic, then it is good to include.


> -- 
> Cheers,
> 
> David

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

[Greg Kroah-Hartman]

On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > >>> 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,
> > > 
> > > Generally:
> > > - The guest is expected to whitelist drivers (most drivers have not
> > >   been audited).
> > 
> > But even if you audited your driver, who makes sure that we consider all ways
> > where the device could mess with us?
> 
> A lot of this is up to a correct setup. For example, make sure all
> filesystems are encrypted and refuse to mount unencrypted ones.
> 
> > Something feels off here.
> > 
> > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > to be corrected.
> 
> Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> 	It is important to note
> 	that this doesn’t imply that the host or VMM are intentionally
> 	malicious, but that there exists a security value in having a small CoCo
> 	VM TCB.
> 
> and
> 
> 	While traditionally the host has unlimited access to guest data and can
> 	leverage this access to attack the guest, the CoCo systems mitigate such
> 	attacks by adding security features like guest data confidentiality and
> 	integrity protection.
> 
> 
> now, when we are talking about "mitigation" it is indeed becoming a bit
> murky.
> 
> 
> For me, a rule of thumb I came up with is that if the validation happens
> to also be helful for users e.g. to work around buggy devices,
> or maybe because we feel failing gracefully is nice because this
> will allow to later make use of this config and old drivers will
> fail but at least not panic, then it is good to include.

Why not do what USB does?  Don't trust the device until AFTER probe()
succeeds?  All of the needed checking should happen before then, as that
is a "slow path" so lots of validation and the like can happen at that
point.

After that, during the normal data paths, after the driver is bound,
trust it all you want as attempting to validate every single packet is
just going to be impossible.

thanks,

greg k-h

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

[Michael S. Tsirkin]

On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > >>> 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,
> > > > 
> > > > Generally:
> > > > - The guest is expected to whitelist drivers (most drivers have not
> > > >   been audited).
> > > 
> > > But even if you audited your driver, who makes sure that we consider all ways
> > > where the device could mess with us?
> > 
> > A lot of this is up to a correct setup. For example, make sure all
> > filesystems are encrypted and refuse to mount unencrypted ones.
> > 
> > > Something feels off here.
> > > 
> > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > to be corrected.
> > 
> > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > 	It is important to note
> > 	that this doesn’t imply that the host or VMM are intentionally
> > 	malicious, but that there exists a security value in having a small CoCo
> > 	VM TCB.
> > 
> > and
> > 
> > 	While traditionally the host has unlimited access to guest data and can
> > 	leverage this access to attack the guest, the CoCo systems mitigate such
> > 	attacks by adding security features like guest data confidentiality and
> > 	integrity protection.
> > 
> > 
> > now, when we are talking about "mitigation" it is indeed becoming a bit
> > murky.
> > 
> > 
> > For me, a rule of thumb I came up with is that if the validation happens
> > to also be helful for users e.g. to work around buggy devices,
> > or maybe because we feel failing gracefully is nice because this
> > will allow to later make use of this config and old drivers will
> > fail but at least not panic, then it is good to include.
> 
> Why not do what USB does?  Don't trust the device until AFTER probe()
> succeeds?  All of the needed checking should happen before then, as that
> is a "slow path" so lots of validation and the like can happen at that
> point.
> 
> After that, during the normal data paths, after the driver is bound,
> trust it all you want as attempting to validate every single packet is
> just going to be impossible.
> 
> thanks,
> 
> greg k-h

People do expect that data path validation at this point.

-- 
MST

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

[Greg Kroah-Hartman]

On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > >>> 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,
> > > > > 
> > > > > Generally:
> > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > >   been audited).
> > > > 
> > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > where the device could mess with us?
> > > 
> > > A lot of this is up to a correct setup. For example, make sure all
> > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > 
> > > > Something feels off here.
> > > > 
> > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > to be corrected.
> > > 
> > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > 	It is important to note
> > > 	that this doesn’t imply that the host or VMM are intentionally
> > > 	malicious, but that there exists a security value in having a small CoCo
> > > 	VM TCB.
> > > 
> > > and
> > > 
> > > 	While traditionally the host has unlimited access to guest data and can
> > > 	leverage this access to attack the guest, the CoCo systems mitigate such
> > > 	attacks by adding security features like guest data confidentiality and
> > > 	integrity protection.
> > > 
> > > 
> > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > murky.
> > > 
> > > 
> > > For me, a rule of thumb I came up with is that if the validation happens
> > > to also be helful for users e.g. to work around buggy devices,
> > > or maybe because we feel failing gracefully is nice because this
> > > will allow to later make use of this config and old drivers will
> > > fail but at least not panic, then it is good to include.
> > 
> > Why not do what USB does?  Don't trust the device until AFTER probe()
> > succeeds?  All of the needed checking should happen before then, as that
> > is a "slow path" so lots of validation and the like can happen at that
> > point.
> > 
> > After that, during the normal data paths, after the driver is bound,
> > trust it all you want as attempting to validate every single packet is
> > just going to be impossible.
> > 
> > thanks,
> > 
> > greg k-h
> 
> People do expect that data path validation at this point.

Ok, so you want this patch :)

And more, as you need to treat everything from the host as "untrusted",
and it must be "verified".  Much like the proposed Rust patches we keep
slowly working on, which would make this much more obvious as to exactly
what needs to be verified, and where it happens.

thanks,

greg k-h

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

[David Hildenbrand (Arm)]

On 7/17/26 12:15, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
>> On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
>>>
>>> Why not do what USB does?  Don't trust the device until AFTER probe()
>>> succeeds?  All of the needed checking should happen before then, as that
>>> is a "slow path" so lots of validation and the like can happen at that
>>> point.
>>>
>>> After that, during the normal data paths, after the driver is bound,
>>> trust it all you want as attempting to validate every single packet is
>>> just going to be impossible.
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> People do expect that data path validation at this point.
> 
> Ok, so you want this patch :)

I fail to see the value of this patch given that there are plenty of other cases
the device can mess with us.

But sure, let's check for some conditions if it makes us feel warm and fluffy as
we audited a driver and it's now super safe, fine with me.

-- 
Cheers,

David

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

[Michael S. Tsirkin]

On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > >>> 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,
> > > > > > 
> > > > > > Generally:
> > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > >   been audited).
> > > > > 
> > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > where the device could mess with us?
> > > > 
> > > > A lot of this is up to a correct setup. For example, make sure all
> > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > 
> > > > > Something feels off here.
> > > > > 
> > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > to be corrected.
> > > > 
> > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > 	It is important to note
> > > > 	that this doesn’t imply that the host or VMM are intentionally
> > > > 	malicious, but that there exists a security value in having a small CoCo
> > > > 	VM TCB.
> > > > 
> > > > and
> > > > 
> > > > 	While traditionally the host has unlimited access to guest data and can
> > > > 	leverage this access to attack the guest, the CoCo systems mitigate such
> > > > 	attacks by adding security features like guest data confidentiality and
> > > > 	integrity protection.
> > > > 
> > > > 
> > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > murky.
> > > > 
> > > > 
> > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > to also be helful for users e.g. to work around buggy devices,
> > > > or maybe because we feel failing gracefully is nice because this
> > > > will allow to later make use of this config and old drivers will
> > > > fail but at least not panic, then it is good to include.
> > > 
> > > Why not do what USB does?  Don't trust the device until AFTER probe()
> > > succeeds?  All of the needed checking should happen before then, as that
> > > is a "slow path" so lots of validation and the like can happen at that
> > > point.
> > > 
> > > After that, during the normal data paths, after the driver is bound,
> > > trust it all you want as attempting to validate every single packet is
> > > just going to be impossible.
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > People do expect that data path validation at this point.
> 
> Ok, so you want this patch :)
> 
> And more, as you need to treat everything from the host as "untrusted",
> and it must be "verified".

Well. First it's not me) Second it's only specific configurations -
for example there's no short term plan to validate filesystem code, people
are expected to rely on encryption. The reasons have more to do
with the available manpower than anything else.



>  Much like the proposed Rust patches we keep
> slowly working on, which would make this much more obvious as to exactly
> what needs to be verified, and where it happens.
> 
> thanks,
> 
> greg k-h

Or much like networking, except it's an old robust codebase.

-- 
MST

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

[Michael S. Tsirkin]

On Fri, Jul 17, 2026 at 11:21:34AM +0100, David Hildenbrand (Arm) wrote:
> On 7/17/26 12:15, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> >> On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> >>>
> >>> Why not do what USB does?  Don't trust the device until AFTER probe()
> >>> succeeds?  All of the needed checking should happen before then, as that
> >>> is a "slow path" so lots of validation and the like can happen at that
> >>> point.
> >>>
> >>> After that, during the normal data paths, after the driver is bound,
> >>> trust it all you want as attempting to validate every single packet is
> >>> just going to be impossible.
> >>>
> >>> thanks,
> >>>
> >>> greg k-h
> >>
> >> People do expect that data path validation at this point.
> > 
> > Ok, so you want this patch :)
> 
> I fail to see the value of this patch given that there are plenty of other cases
> the device can mess with us.
> 
> But sure, let's check for some conditions if it makes us feel warm and fluffy as
> we audited a driver and it's now super safe, fine with me.
> 
> -- 
> Cheers,
> 
> David

I merely made some generic comments since I was asked to clarify how
virtio interacts with coco.  As for the specific patch, it does not make
me feel fluffy. It lacks motivation.

A broken device confuses the kernel with no way to exploit that. Shrug. So?
Nothing to do with coco, apparently.

It's maybe reasonable as a debugging aid but if this is intended as such
let's make it clear in the commit log.

-- 
MST

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

[Greg Kroah-Hartman]

On Fri, Jul 17, 2026 at 11:21:34AM +0100, David Hildenbrand (Arm) wrote:
> On 7/17/26 12:15, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> >> On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> >>>
> >>> Why not do what USB does?  Don't trust the device until AFTER probe()
> >>> succeeds?  All of the needed checking should happen before then, as that
> >>> is a "slow path" so lots of validation and the like can happen at that
> >>> point.
> >>>
> >>> After that, during the normal data paths, after the driver is bound,
> >>> trust it all you want as attempting to validate every single packet is
> >>> just going to be impossible.
> >>>
> >>> thanks,
> >>>
> >>> greg k-h
> >>
> >> People do expect that data path validation at this point.
> > 
> > Ok, so you want this patch :)
> 
> I fail to see the value of this patch given that there are plenty of other cases
> the device can mess with us.

How can the device mess with us?

> But sure, let's check for some conditions if it makes us feel warm and fluffy as
> we audited a driver and it's now super safe, fine with me.

I'm all for the folly of "it's an audited driver!" claims, but you all
need to decide either you do or you do not trust the device.  Either way
is fine with me.

If you don't trust it, great, take patches that fix that.  If you do
trust it, great, reject those types of patches.

But pick one please.

thanks,

greg k-h

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

[Greg Kroah-Hartman]

On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
> On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > > >>> 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,
> > > > > > > 
> > > > > > > Generally:
> > > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > >   been audited).
> > > > > > 
> > > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > > where the device could mess with us?
> > > > > 
> > > > > A lot of this is up to a correct setup. For example, make sure all
> > > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > > 
> > > > > > Something feels off here.
> > > > > > 
> > > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > > to be corrected.
> > > > > 
> > > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > > 	It is important to note
> > > > > 	that this doesn’t imply that the host or VMM are intentionally
> > > > > 	malicious, but that there exists a security value in having a small CoCo
> > > > > 	VM TCB.
> > > > > 
> > > > > and
> > > > > 
> > > > > 	While traditionally the host has unlimited access to guest data and can
> > > > > 	leverage this access to attack the guest, the CoCo systems mitigate such
> > > > > 	attacks by adding security features like guest data confidentiality and
> > > > > 	integrity protection.
> > > > > 
> > > > > 
> > > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > > murky.
> > > > > 
> > > > > 
> > > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > > to also be helful for users e.g. to work around buggy devices,
> > > > > or maybe because we feel failing gracefully is nice because this
> > > > > will allow to later make use of this config and old drivers will
> > > > > fail but at least not panic, then it is good to include.
> > > > 
> > > > Why not do what USB does?  Don't trust the device until AFTER probe()
> > > > succeeds?  All of the needed checking should happen before then, as that
> > > > is a "slow path" so lots of validation and the like can happen at that
> > > > point.
> > > > 
> > > > After that, during the normal data paths, after the driver is bound,
> > > > trust it all you want as attempting to validate every single packet is
> > > > just going to be impossible.
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > 
> > > People do expect that data path validation at this point.
> > 
> > Ok, so you want this patch :)
> > 
> > And more, as you need to treat everything from the host as "untrusted",
> > and it must be "verified".
> 
> Well. First it's not me) Second it's only specific configurations -
> for example there's no short term plan to validate filesystem code, people
> are expected to rely on encryption. The reasons have more to do
> with the available manpower than anything else.

Sure, but again, for subsystems, you have to define your threat model as
the LLMs are churning against the code base and coming up with lots of
crazy ideas if a device should or should not be trusted and spitting out
patches and reports like the ones that are in the first few patches of
this series.

So please, pick a model, let's document it, and go with that.  I am
getting directly conflicting responses here.

thanks,

greg k-h

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

[Michael S. Tsirkin]

On Fri, Jul 17, 2026 at 12:46:52PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > >>> 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,
> > > > > > > > 
> > > > > > > > Generally:
> > > > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > > >   been audited).
> > > > > > > 
> > > > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > > > where the device could mess with us?
> > > > > > 
> > > > > > A lot of this is up to a correct setup. For example, make sure all
> > > > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > > > 
> > > > > > > Something feels off here.
> > > > > > > 
> > > > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > > > to be corrected.
> > > > > > 
> > > > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > > > 	It is important to note
> > > > > > 	that this doesn’t imply that the host or VMM are intentionally
> > > > > > 	malicious, but that there exists a security value in having a small CoCo
> > > > > > 	VM TCB.
> > > > > > 
> > > > > > and
> > > > > > 
> > > > > > 	While traditionally the host has unlimited access to guest data and can
> > > > > > 	leverage this access to attack the guest, the CoCo systems mitigate such
> > > > > > 	attacks by adding security features like guest data confidentiality and
> > > > > > 	integrity protection.
> > > > > > 
> > > > > > 
> > > > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > > > murky.
> > > > > > 
> > > > > > 
> > > > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > > > to also be helful for users e.g. to work around buggy devices,
> > > > > > or maybe because we feel failing gracefully is nice because this
> > > > > > will allow to later make use of this config and old drivers will
> > > > > > fail but at least not panic, then it is good to include.
> > > > > 
> > > > > Why not do what USB does?  Don't trust the device until AFTER probe()
> > > > > succeeds?  All of the needed checking should happen before then, as that
> > > > > is a "slow path" so lots of validation and the like can happen at that
> > > > > point.
> > > > > 
> > > > > After that, during the normal data paths, after the driver is bound,
> > > > > trust it all you want as attempting to validate every single packet is
> > > > > just going to be impossible.
> > > > > 
> > > > > thanks,
> > > > > 
> > > > > greg k-h
> > > > 
> > > > People do expect that data path validation at this point.
> > > 
> > > Ok, so you want this patch :)
> > > 
> > > And more, as you need to treat everything from the host as "untrusted",
> > > and it must be "verified".
> > 
> > Well. First it's not me) Second it's only specific configurations -
> > for example there's no short term plan to validate filesystem code, people
> > are expected to rely on encryption. The reasons have more to do
> > with the available manpower than anything else.
> 
> Sure, but again, for subsystems, you have to define your threat model as
> the LLMs are churning against the code base and coming up with lots of
> crazy ideas if a device should or should not be trusted and spitting out
> patches and reports like the ones that are in the first few patches of
> this series.
> 
> So please, pick a model, let's document it, and go with that.  I am
> getting directly conflicting responses here.
> 
> thanks,
> 
> greg k-h

Supposed to be this one:
Documentation/security/snp-tdx-threat-model.rst

what is missing?


-- 
MST

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

[David Hildenbrand (Arm)]

On 7/17/26 12:44, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 11:21:34AM +0100, David Hildenbrand (Arm) wrote:
>> On 7/17/26 12:15, Greg Kroah-Hartman wrote:
>>>
>>> Ok, so you want this patch :)
>>
>> I fail to see the value of this patch given that there are plenty of other cases
>> the device can mess with us.
> 
> How can the device mess with us?

I raised some examples already, like lying about which memory chunks it makes
available.

I guess it could also throw a random "requested_size" at us. It could make up a
wrong physical address region. I assume there is plenty more we'd have to check.

Again, I'm not saying that this couldn't/shouldn't be done, but it requires some
*real thought* about all possible things a device could do.

> 
>> But sure, let's check for some conditions if it makes us feel warm and fluffy as
>> we audited a driver and it's now super safe, fine with me.
> 
> I'm all for the folly of "it's an audited driver!" claims, but you all
> need to decide either you do or you do not trust the device.  Either way
> is fine with me.

Well, exactly, that is what I am saying. I don't know what we care about. This
patch here feels incomplete and that's what grinds my gears. It doesn't
magically make us deal with malicious devices.

And I don't buy the story about "buggy virtio-mem devices that set
block_size==0", which doesn't make any sense in any possible reality.

> 
> If you don't trust it, great, take patches that fix that.  If you do
> trust it, great, reject those types of patches.
> 
> But pick one please.

Yes, I'd expect that we have general virtio guifance. I only co-maintain some
virtio bits and have no idea about the expected trust model and when we would
consider a devices trusted.

And what it would take to get there.

-- 
Cheers,

David