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

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

[Michael Bommarito]

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;
 }
-- 
2.53.0

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

[Christian Schoenebeck]

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

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

[Michael Bommarito]

On Wed, Jun 10, 2026 at 11:46 AM Christian Schoenebeck
<linux_oss@crudebyte.com> wrote:
> 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?

I think that's much cleaner if the user base really can't think of any
legitimate purpose here (or in userspace utils).

FWIW, on a similar Xen issue, we're also talking about blacklisting,
which is one step even further.

Thanks,
Mike

[PATCH v3] 9p/trans_virtio: reject mount tags that cannot fit a sysfs page

[Michael Bommarito]

p9_virtio_probe() reads the mount tag length from the device's
virtio_9p_config.tag_len, a 16-bit field, and accepts any value up to
65535 with no upper bound:

	virtio_cread(vdev, struct virtio_9p_config, tag_len, &tag_len);
	...
	tag = kzalloc(tag_len + 1, GFP_KERNEL);

The tag is later emitted through the world-readable
/sys/.../mount_tag attribute by p9_mount_tag_show(), which copies the
whole NUL-terminated tag into the single PAGE_SIZE buffer that the
sysfs core provides:

	tag_len = strlen(chan->tag);
	memcpy(buf, chan->tag, tag_len + 1);

A tag longer than the page therefore overruns the sysfs buffer. Under
the confidential-computing threat model, where the guest does not
trust the host, a malicious or compromised host can advertise a
~64 KiB tag; the first read of mount_tag (udev reads it at probe) then
copies host-controlled content past the end of the 4 KiB page, a slab
out-of-bounds write.

A tag that large can never be rendered through the single-page sysfs
attribute and is not usable as a real mount tag, so a tag at or beyond
PAGE_SIZE is at best malfunctioning and at worst hostile. Reject such a
device at probe time rather than truncating the value in the show
handler, which would silently break auto-mount rules that match on the
real tag. The probe-time bound makes the existing p9_mount_tag_show()
copy safe at its source, so the show handler is left unchanged.

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>
---
v3: Reject an oversized tag at probe time instead of touching the
    show handler at all, following Christian Schoenebeck's review of
    v1: a sane 9p device would never use a ~64 KiB tag, silently
    truncating it (as v1, and as the held v2 sysfs_emit() reroll both
    did) would break auto-mount rules that match on the real tag, so
    the device is simply refused. This supersedes the v2 sysfs_emit()
    patch, which is dropped in favour of the probe-time bound
    Schoenebeck preferred.
    v1: https://lore.kernel.org/v9fs/20260610114206.3749904-1-michael.bommarito@gmail.com/
    review: https://lore.kernel.org/v9fs/1962500.CQOukoFCf9@weasel/

Compile-tested (ARCH=um, W=1) on torvalds/master, no new warnings.
The original overflow was reproduced before the fix with an in-tree
KUnit driver under UML+KASAN (a 65535-byte non-NUL tag drove a KASAN
slab-out-of-bounds write into the 4 KiB sysfs page). With this patch
the probe rejects such a device, so the show handler never sees an
out-of-page tag and the memcpy stays in bounds.

 net/9p/trans_virtio.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index b0d0094ec8e2c..15568f40509c4 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -633,6 +633,11 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 		err = -EINVAL;
 		goto out_free_vq;
 	}
+	if (tag_len >= PAGE_SIZE) {
+		dev_err(&vdev->dev, "mount tag too long (%u bytes)\n", tag_len);
+		err = -EINVAL;
+		goto out_free_vq;
+	}
 	tag = kzalloc(tag_len + 1, GFP_KERNEL);
 	if (!tag) {
 		err = -ENOMEM;

base-commit: 4edcdefd4083ae04b1a5656f4be6cd83ae919ef4
-- 
2.53.0

Re: [PATCH v3] 9p/trans_virtio: reject mount tags that cannot fit a sysfs page

[Christian Schoenebeck]

On Friday, 26 June 2026 13:19:06 CEST Michael Bommarito wrote:
> p9_virtio_probe() reads the mount tag length from the device's
> virtio_9p_config.tag_len, a 16-bit field, and accepts any value up to
> 65535 with no upper bound:
> 
> 	virtio_cread(vdev, struct virtio_9p_config, tag_len, &tag_len);
> 	...
> 	tag = kzalloc(tag_len + 1, GFP_KERNEL);
> 
> The tag is later emitted through the world-readable
> /sys/.../mount_tag attribute by p9_mount_tag_show(), which copies the
> whole NUL-terminated tag into the single PAGE_SIZE buffer that the
> sysfs core provides:
> 
> 	tag_len = strlen(chan->tag);
> 	memcpy(buf, chan->tag, tag_len + 1);
> 
> A tag longer than the page therefore overruns the sysfs buffer. Under
> the confidential-computing threat model, where the guest does not
> trust the host, a malicious or compromised host can advertise a
> ~64 KiB tag; the first read of mount_tag (udev reads it at probe) then
> copies host-controlled content past the end of the 4 KiB page, a slab
> out-of-bounds write.
[...]
>  net/9p/trans_virtio.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
> index b0d0094ec8e2c..15568f40509c4 100644
> --- a/net/9p/trans_virtio.c
> +++ b/net/9p/trans_virtio.c
> @@ -633,6 +633,11 @@ static int p9_virtio_probe(struct virtio_device *vdev)
>  		err = -EINVAL;
>  		goto out_free_vq;
>  	}
> +	if (tag_len >= PAGE_SIZE) {

That should be tag_len >= PAGE_SIZE - 1.

However you are still assuming and hard-coding an implementation specific
limit of seq_file. If that limit changes there, it breaks *silently* here
again.

I would rather handle this more aggressively by using NAME_MAX (255) as tag
length limit; safe and avoids complicated alternative solutions.

> +		dev_err(&vdev->dev, "mount tag too long (%u bytes)\n", tag_len);
> +		err = -EINVAL;
> +		goto out_free_vq;
> +	}
>  	tag = kzalloc(tag_len + 1, GFP_KERNEL);
>  	if (!tag) {
>  		err = -ENOMEM;
> 
> base-commit: 4edcdefd4083ae04b1a5656f4be6cd83ae919ef4