Re: [PATCH] 9p/trans_virtio: bound mount_tag show copy to one page

[Christian Schoenebeck <linux_oss@crudebyte.com>]

On Wednesday, 10 June 2026 13:42:06 CEST Michael Bommarito wrote:
> p9_mount_tag_show() copies strlen(chan->tag) + 1 bytes into the
> single-page buffer the sysfs core provides, with no upper bound. The
> mount tag length comes from virtio_9p_config.tag_len, a 16-bit field read
> from the device at probe in p9_virtio_probe() with no cap. Under the
> confidential-computing threat model, where the guest does not trust the
> host, a malicious or compromised host can present a 65535-byte tag with
> no embedded NUL. A read of the world-readable /sys/.../mount_tag
> attribute (udev reads it at probe) then copies ~64 KiB into the 4 KiB
> sysfs page, a slab-out-of-bounds write of host-controlled content.
> 
> Bound the copy to the page size in the show handler.
> 
> Fixes: 179a5bc4b8cb ("net/9p: use memcpy() instead of snprintf() in
> p9_mount_tag_show()") Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  net/9p/trans_virtio.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
> index 4cdab7094b273..b62aa7b309f1c 100644
> --- a/net/9p/trans_virtio.c
> +++ b/net/9p/trans_virtio.c
> @@ -573,7 +573,11 @@ static ssize_t p9_mount_tag_show(struct device *dev,
>  	chan = vdev->priv;
>  	tag_len = strlen(chan->tag);
> 
> -	memcpy(buf, chan->tag, tag_len + 1);
> +	if (tag_len > PAGE_SIZE - 2)
> +		tag_len = PAGE_SIZE - 2;
> +
> +	memcpy(buf, chan->tag, tag_len);
> +	buf[tag_len] = '\0';
> 
>  	return tag_len + 1;
>  }

Given that this has already seen some rotations, it is worth to think a bit 
more about it:

1. Destination buffer being PAGE_SIZE large is an implementation detail of 
seq_file. If the latter is changed, this code breaks (again) silently without 
anybody noticing.

2. memcpy() was introduced by 179a5bc4b8cb because chan->tag was not NULL 
terminated. However since edcd9d977354 it *is* NULL terminated.

Given those two, an alternative would be:

  len = sysfs_emit(buf, "%s", chan->tag);

As it already handles the PAGE_SIZE limit inside its implementation.

However, still ...

3. Is it a good idea to just *silenty* truncate a very long tag to something 
else just to avoid an OOB? It would at least break auto mount rules, as the 
truncated tag would not match device's real tag.

4. And most importantly: Would a sane 9p device *ever* use a 64k tag, if yes 
what for?

I would say no, a 64k tag is at least suspicious, if not even hostile.

Therefore: what about simply rejecting the device at probe time if its tag is 
beyond a certain length?

/Christian