Re: [PATCH 1/1] 9p: fix caches_show() out-of-bounds write

[Dominique Martinet <asmadeus@codewreck.org>]

Ren Wei wrote on Sat, Jun 20, 2026 at 11:53:03PM +0800:
> From: Zhao Zhang <zzhan461@ucr.edu>
> 
> The sysfs show handler for /sys/fs/9p/caches appends cache tags with
> snprintf(buf + count, limit, ...) and then advances count and limit by
> the return value. This is incorrect for truncation because snprintf()
> returns the full would-have-been length, not the number of bytes stored.
> 
> Once the accumulated output exceeds PAGE_SIZE, count can advance past
> the sysfs buffer and limit can become negative. A later iteration then
> passes an out-of-bounds destination pointer and an oversized size_t
> into snprintf(), leading to an out-of-bounds write.
> 
> Use sysfs_emit_at() for the append instead. It follows the sysfs buffer
> contract and returns the number of bytes actually stored, so the offset
> remains bounded even when the output is truncated.
> 
> Fixes: 86db0c32f16c ("9p: fix /sys/fs/9p/caches overwriting itself")
> Cc: stable@vger.kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Zhengchuan Liang <zcliangcn@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Assisted-by: Codex:GPT-5.4
> Signed-off-by: Zhao Zhang <zzhan461@ucr.edu>

nit: this is missing a Co-authored-by tag between the two sign-offs if
you touched the patch, or should not have your sign-off at all if you
didn't, see Documentation/process/submitting-patches.rst

Co-authored-by: Ren Wei <n05ec@lzu.edu.cn>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>

Less minor nit: please always cc linux-kernel@vger.kernel.org when
sending patches; it should come up if you use get_maintainer.pl
(In this case I'd also add fsdevel but that doesn't get listed for some
reason, so I guess I can't argue about that one; doesn't really matter
for something simple as this though, but not having linux-kernel@ means
sashiko didn't run)

> ---
>  fs/9p/v9fs.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index acda42499ca9..0668bad681bf 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -17,6 +17,7 @@
>  #include <linux/fs_context.h>
>  #include <linux/slab.h>
>  #include <linux/seq_file.h>
> +#include <linux/sysfs.h>
>  #include <net/9p/9p.h>
>  #include <net/9p/client.h>
>  #include <net/9p/transport.h>
> @@ -592,21 +593,14 @@ static ssize_t caches_show(struct kobject *kobj,
>  			   struct kobj_attribute *attr,
>  			   char *buf)
>  {
> -	ssize_t n = 0, count = 0, limit = PAGE_SIZE;
> +	ssize_t count = 0;
>  	struct v9fs_session_info *v9ses;
>  
>  	spin_lock(&v9fs_sessionlist_lock);
>  	list_for_each_entry(v9ses, &v9fs_sessionlist, slist) {
> -		if (v9ses->cachetag) {
> -			n = snprintf(buf + count, limit, "%s\n", v9ses->cachetag);
> -			if (n < 0) {
> -				count = n;
> -				break;
> -			}
> -
> -			count += n;
> -			limit -= n;
> -		}
> +		if (v9ses->cachetag)
> +			count += sysfs_emit_at(buf, count, "%s\n",
> +					       v9ses->cachetag);

This makes sense to me, there's just a nit: looking at sysfs_emit_at()
if count is already >= PAGE_SIZE then this emits a WARN(), which can be
problematic for some people;
I think we should explicitly break if count >= PAGE_SIZE to avoid this


>  	}
>  
>  	spin_unlock(&v9fs_sessionlist_lock);

-- 
Dominique Martinet | Asmadeus