[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

[PATCH v4] 9p/trans_virtio: reject mount tags longer than NAME_MAX at probe

[Michael Bommarito]

p9_virtio_probe() reads a 16-bit mount tag length (tag_len) from the
device config and kzalloc()s tag_len + 1 bytes for chan->tag with no
upper bound. A malicious or compromised host can present a tag of up to
65535 bytes; that tag is later copied into the single-page sysfs buffer
by p9_mount_tag_show() (memcpy(buf, chan->tag, tag_len + 1)), a
host-controlled out-of-bounds write of up to ~64 KiB past the PAGE_SIZE
attribute buffer.

Reject an overlong tag at probe time. A 9p mount tag is a name-like
identifier, so bound it by NAME_MAX (255): that limit is independent of
the sysfs/seq_file page size and is far above any legitimate mount tag,
which keeps p9_mount_tag_show() safe at the root without hard-coding a
seq_file implementation detail.

Fixes: 179a5bc4b8cb ("net/9p: use memcpy() instead of snprintf() in p9_mount_tag_show()")
Cc: stable@vger.kernel.org
Suggested-by: Christian Schoenebeck <linux_oss@crudebyte.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v4: bound tag_len by NAME_MAX (255) instead of PAGE_SIZE, per Christian
    Schoenebeck's review of v3 -- a 9p mount tag is a name-like identifier, and
    NAME_MAX avoids hard-coding the seq_file page-size implementation detail
    (which could change and silently break the bound) and resolves the
    off-by-one he noted.
v3: reject at probe time (PAGE_SIZE) instead of truncating in show().
v2: sysfs_emit() in show() -- dropped; probe-time rejection preferred.
v1: bound the show() copy.

Build-tested ARCH=um (net/9p/trans_virtio.o, W=1, clean; NAME_MAX resolves with
no new include). The original ~64 KiB OOB write was reproduced pre-fix; with the
probe bound a tag_len > 255 device is refused, so the show handler never sees an
out-of-page tag.

 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..9fa63e3ebf673 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 > NAME_MAX) {
+		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 v4] 9p/trans_virtio: reject mount tags longer than NAME_MAX at probe

[Christian Schoenebeck]

On Saturday, 27 June 2026 23:10:12 CEST Michael Bommarito wrote:
> p9_virtio_probe() reads a 16-bit mount tag length (tag_len) from the
> device config and kzalloc()s tag_len + 1 bytes for chan->tag with no
> upper bound. A malicious or compromised host can present a tag of up to
> 65535 bytes; that tag is later copied into the single-page sysfs buffer
> by p9_mount_tag_show() (memcpy(buf, chan->tag, tag_len + 1)), a
> host-controlled out-of-bounds write of up to ~64 KiB past the PAGE_SIZE
> attribute buffer.
> 
> Reject an overlong tag at probe time. A 9p mount tag is a name-like
> identifier, so bound it by NAME_MAX (255): that limit is independent of
> the sysfs/seq_file page size and is far above any legitimate mount tag,
> which keeps p9_mount_tag_show() safe at the root without hard-coding a
> seq_file implementation detail.
> 
> Fixes: 179a5bc4b8cb ("net/9p: use memcpy() instead of snprintf() in
> p9_mount_tag_show()") Cc: stable@vger.kernel.org
> Suggested-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>

Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com>

> ---
> v4: bound tag_len by NAME_MAX (255) instead of PAGE_SIZE, per Christian
>     Schoenebeck's review of v3 -- a 9p mount tag is a name-like identifier,
> and NAME_MAX avoids hard-coding the seq_file page-size implementation
> detail (which could change and silently break the bound) and resolves the
> off-by-one he noted.
> v3: reject at probe time (PAGE_SIZE) instead of truncating in show().
> v2: sysfs_emit() in show() -- dropped; probe-time rejection preferred.
> v1: bound the show() copy.
> 
> Build-tested ARCH=um (net/9p/trans_virtio.o, W=1, clean; NAME_MAX resolves
> with no new include). The original ~64 KiB OOB write was reproduced
> pre-fix; with the probe bound a tag_len > 255 device is refused, so the
> show handler never sees an out-of-page tag.
> 
>  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..9fa63e3ebf673 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 > NAME_MAX) {
> +		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

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

[kernel test robot]

Hi Michael,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 4edcdefd4083ae04b1a5656f4be6cd83ae919ef4]

url:    https://github.com/intel-lab-lkp/linux/commits/Michael-Bommarito/9p-trans_virtio-reject-mount-tags-that-cannot-fit-a-sysfs-page/20260629-093057
base:   4edcdefd4083ae04b1a5656f4be6cd83ae919ef4
patch link:    https://lore.kernel.org/r/20260626111906.801890-1-michael.bommarito%40gmail.com
patch subject: [PATCH v3] 9p/trans_virtio: reject mount tags that cannot fit a sysfs page
config: arm64-randconfig-r063-20260629 (https://download.01.org/0day-ci/archive/20260629/202606291938.9FOZRc5c-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260629/202606291938.9FOZRc5c-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606291938.9FOZRc5c-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/9p/trans_virtio.c:636:14: warning: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Wtautological-constant-out-of-range-compare]
     636 |         if (tag_len >= PAGE_SIZE) {
         |             ~~~~~~~ ^  ~~~~~~~~~
   1 warning generated.


vim +636 net/9p/trans_virtio.c

   587	
   588	/**
   589	 * p9_virtio_probe - probe for existence of 9P virtio channels
   590	 * @vdev: virtio device to probe
   591	 *
   592	 * This probes for existing virtio channels.
   593	 *
   594	 */
   595	
   596	static int p9_virtio_probe(struct virtio_device *vdev)
   597	{
   598		__u16 tag_len;
   599		char *tag;
   600		int err;
   601		struct virtio_chan *chan;
   602	
   603		if (!vdev->config->get) {
   604			dev_err(&vdev->dev, "%s failure: config access disabled\n",
   605				__func__);
   606			return -EINVAL;
   607		}
   608	
   609		chan = kmalloc_obj(struct virtio_chan);
   610		if (!chan) {
   611			pr_err("Failed to allocate virtio 9P channel\n");
   612			err = -ENOMEM;
   613			goto fail;
   614		}
   615	
   616		chan->vdev = vdev;
   617	
   618		/* We expect one virtqueue, for requests. */
   619		chan->vq = virtio_find_single_vq(vdev, req_done, "requests");
   620		if (IS_ERR(chan->vq)) {
   621			err = PTR_ERR(chan->vq);
   622			goto out_free_chan;
   623		}
   624		chan->vq->vdev->priv = chan;
   625		spin_lock_init(&chan->lock);
   626	
   627		sg_init_table(chan->sg, VIRTQUEUE_NUM);
   628	
   629		chan->inuse = false;
   630		if (virtio_has_feature(vdev, VIRTIO_9P_MOUNT_TAG)) {
   631			virtio_cread(vdev, struct virtio_9p_config, tag_len, &tag_len);
   632		} else {
   633			err = -EINVAL;
   634			goto out_free_vq;
   635		}
 > 636		if (tag_len >= PAGE_SIZE) {
   637			dev_err(&vdev->dev, "mount tag too long (%u bytes)\n", tag_len);
   638			err = -EINVAL;
   639			goto out_free_vq;
   640		}
   641		tag = kzalloc(tag_len + 1, GFP_KERNEL);
   642		if (!tag) {
   643			err = -ENOMEM;
   644			goto out_free_vq;
   645		}
   646	
   647		virtio_cread_bytes(vdev, offsetof(struct virtio_9p_config, tag),
   648				   tag, tag_len);
   649		chan->tag = tag;
   650		err = sysfs_create_file(&(vdev->dev.kobj), &dev_attr_mount_tag.attr);
   651		if (err) {
   652			goto out_free_tag;
   653		}
   654		chan->vc_wq = kmalloc_obj(wait_queue_head_t);
   655		if (!chan->vc_wq) {
   656			err = -ENOMEM;
   657			goto out_remove_file;
   658		}
   659		init_waitqueue_head(chan->vc_wq);
   660		chan->ring_bufs_avail = 1;
   661		/* Ceiling limit to avoid denial of service attacks */
   662		chan->p9_max_pages = nr_free_buffer_pages()/4;
   663	
   664		virtio_device_ready(vdev);
   665	
   666		mutex_lock(&virtio_9p_lock);
   667		list_add_tail(&chan->chan_list, &virtio_chan_list);
   668		mutex_unlock(&virtio_9p_lock);
   669	
   670		/* Let udev rules use the new mount_tag attribute. */
   671		kobject_uevent(&(vdev->dev.kobj), KOBJ_CHANGE);
   672	
   673		return 0;
   674	
   675	out_remove_file:
   676		sysfs_remove_file(&vdev->dev.kobj, &dev_attr_mount_tag.attr);
   677	out_free_tag:
   678		kfree(tag);
   679	out_free_vq:
   680		vdev->config->del_vqs(vdev);
   681	out_free_chan:
   682		kfree(chan);
   683	fail:
   684		return err;
   685	}
   686	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki