public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tyler Hicks <tyhicks@linux.microsoft.com>
To: Eric Van Hensbergen <ericvh@gmail.com>,
	Latchesar Ionkov <lucho@ionkov.net>,
	Dominique Martinet <asmadeus@codewreck.org>,
	Christian Schoenebeck <linux_oss@crudebyte.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	v9fs-developer@lists.sourceforge.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH] net/9p: Initialize the iounit field during fid creation
Date: Sat,  9 Jul 2022 15:00:05 -0500	[thread overview]
Message-ID: <20220709200005.681861-1-tyhicks@linux.microsoft.com> (raw)

Ensure that the fid's iounit field is set to zero when a new fid is
created. Certain 9P operations, such as OPEN and CREATE, allow the
server to reply with an iounit size which the client code assigns to the
fid struct shortly after the fid is created in p9_fid_create(). Other
operations that follow a call to p9_fid_create(), such as an XATTRWALK,
don't include an iounit value in the reply message from the server. In
the latter case, the iounit field remained uninitialized. Depending on
allocation patterns, the iounit value could have been something
reasonable that was carried over from previously freed fids or, in the
worst case, could have been arbitrary values from non-fid related usages
of the memory location.

The bug was detected in the Windows Subsystem for Linux 2 (WSL2) kernel
after the uninitialized iounit field resulted in the typical sequence of
two getxattr(2) syscalls, one to get the size of an xattr and another
after allocating a sufficiently sized buffer to fit the xattr value, to
hit an unexpected ERANGE error in the second call to getxattr(2). An
uninitialized iounit field would sometimes force rsize to be smaller
than the xattr value size in p9_client_read_once() and the 9P server in
WSL refused to chunk up the READ on the attr_fid and, instead, returned
ERANGE to the client. The virtfs server in QEMU seems happy to chunk up
the READ and this problem goes undetected there. However, there are
likely other non-xattr implications of this bug that could cause
inefficient communication between the client and server.

Cc: stable@vger.kernel.org
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
---

Note that I haven't had a chance to identify when this bug was
introduced so I don't yet have a proper Fixes tag. The history looked a
little tricky to me but I'll have another look in the coming days. We
started hitting this bug after trying to move from linux-5.10.y to
linux-5.15.y but I didn't see any obvious changes between those two
series. I'm not confident of this theory but perhaps the fid refcounting
changes impacted the fid allocation patterns enough to uncover the
latent bug?

 net/9p/client.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/9p/client.c b/net/9p/client.c
index 8bba0d9cf975..1dfceb9154f7 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -899,6 +899,7 @@ static struct p9_fid *p9_fid_create(struct p9_client *clnt)
 	fid->clnt = clnt;
 	fid->rdir = NULL;
 	fid->fid = 0;
+	fid->iounit = 0;
 	refcount_set(&fid->count, 1);
 
 	idr_preload(GFP_KERNEL);
-- 
2.25.1


             reply	other threads:[~2022-07-09 20:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-09 20:00 Tyler Hicks [this message]
2022-07-10  6:25 ` [PATCH] net/9p: Initialize the iounit field during fid creation Tyler Hicks
2022-07-10  6:38   ` Tyler Hicks
2022-07-10  6:45 ` Christophe JAILLET
2022-07-10 13:21 ` Dominique Martinet
2022-07-10 14:12   ` Tyler Hicks

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220709200005.681861-1-tyhicks@linux.microsoft.com \
    --to=tyhicks@linux.microsoft.com \
    --cc=asmadeus@codewreck.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=ericvh@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=v9fs-developer@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox