[PATCH 0/7] usb9pfs: stable fixes

[PATCH 0/7] usb9pfs: stable fixes

[Michael Grzeschik]

This is a breakout series with some additional fixes of the
first series of fixes I did sent out earlier this year.

https://lore.kernel.org/all/20260319-9pfixes-v1-0-c977a7433185@pengutronix.de/

It only addresses the stability of the driver and left out
the interface rework. Therefor I put all changes into a new
series beginning as v1.

Signed-off-by: Michael Grzeschik <mgr@kernel.org>
---
Hyungjung Joo (1):
      net/9p/usbg: clear stale client pointer on close

Michael Grzeschik (6):
      net/9p/usbg: also disable endpoints on p9_usbg_close
      net/9p/usbg: set client to Disconnected on usb9pfs_disable
      net/9p/usbg: always reset completion when disconnecting
      net/9p/usbg: call disable_usb9pfs() from usb9pfs_disable()
      net/9p/usbg: fix out_req buffer leak in disable_usb9pfs
      net/9p/usbg: remove bogus context initialization in alloc_requests

 net/9p/trans_usbg.c | 80 +++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 59 insertions(+), 21 deletions(-)
---
base-commit: a170f3dfda02b6ab58bcd1caa1ac5421323a9b45
change-id: 20260922-usb9pfsfixes-e3bbdd821fc8

Best regards,
--  
Michael Grzeschik <mgr@kernel.org>

[PATCH 1/7] net/9p/usbg: also disable endpoints on p9_usbg_close

[Michael Grzeschik]

The close function has to fully reverse the state change of 9p_create
(mount) and the potential call of set_alt(1). This includes to ensure
that the usage of the endpoints is not active any more.

Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Cc: stable@vger.kernel.org
Signed-off-by: Michael Grzeschik <mgr@kernel.org>
---
 net/9p/trans_usbg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index 9fd7801d56ab..32e18077309a 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -473,6 +473,8 @@ static void p9_usbg_close(struct p9_client *client)
 	mutex_lock(&usb9pfs_lock);
 	dev->inuse = false;
 	mutex_unlock(&usb9pfs_lock);
+
+	disable_usb9pfs(usb9pfs);
 }
 
 static int p9_usbg_request(struct p9_client *client, struct p9_req_t *p9_req)

-- 
2.53.0

[PATCH 2/7] net/9p/usbg: set client to Disconnected on usb9pfs_disable

[Michael Grzeschik]

This patch is setting the client status to Disconnected, when the
client is still in use. Otherwiese a disconnected usb cable would run
any use of the mount to faults.

Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Cc: stable@vger.kernel.org
Signed-off-by: Michael Grzeschik <mgr@kernel.org>
---
 net/9p/trans_usbg.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index 32e18077309a..a3182d3db175 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -757,7 +757,12 @@ static int usb9pfs_set_alt(struct usb_function *f,
 static void usb9pfs_disable(struct usb_function *f)
 {
 	struct f_usb9pfs *usb9pfs = func_to_usb9pfs(f);
+	unsigned long flags;
 
+	spin_lock_irqsave(&usb9pfs->lock, flags);
+	if (usb9pfs->client)
+		usb9pfs->client->status = Disconnected;
+	spin_unlock_irqrestore(&usb9pfs->lock, flags);
 	usb9pfs_clear_tx(usb9pfs);
 }
 

-- 
2.53.0

[PATCH 3/7] net/9p/usbg: always reset completion when disconnecting

[Michael Grzeschik]

When some tx or rx transfers were pending while closing the connection,
the completion handler could catch one pending completion call. To
ensure a normal start when mounting again, we have to reset the
completion and flush any pending completions.

Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Cc: stable@vger.kernel.org
Signed-off-by: Michael Grzeschik <mgr@kernel.org>
---
 net/9p/trans_usbg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index a3182d3db175..e3af8e1002d7 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -475,6 +475,7 @@ static void p9_usbg_close(struct p9_client *client)
 	mutex_unlock(&usb9pfs_lock);
 
 	disable_usb9pfs(usb9pfs);
+	reinit_completion(&usb9pfs->send);
 }
 
 static int p9_usbg_request(struct p9_client *client, struct p9_req_t *p9_req)
@@ -764,6 +765,7 @@ static void usb9pfs_disable(struct usb_function *f)
 		usb9pfs->client->status = Disconnected;
 	spin_unlock_irqrestore(&usb9pfs->lock, flags);
 	usb9pfs_clear_tx(usb9pfs);
+	reinit_completion(&usb9pfs->send);
 }
 
 static struct usb_function *usb9pfs_alloc(struct usb_function_instance *fi)

-- 
2.53.0

[PATCH 4/7] net/9p/usbg: call disable_usb9pfs() from usb9pfs_disable()

[Michael Grzeschik]

usb9pfs_disable() is the usb_function ->disable callback, invoked by
the composite gadget core whenever the host resets or unconfigures
the gadget. The gadget function API requires this callback to disable
its endpoints; every other in-tree function driver does so from its
->disable hook.

usb9pfs_disable() never called disable_usb9pfs(), so the IN/OUT
endpoints and their usb_request objects were left active and
allocated across a host-driven disable. When the host later
reconfigures the device, enable_usb9pfs() calls alloc_requests() again
and unconditionally overwrites usb9pfs->in_req/out_req, permanently
leaking the previous allocations, while the endpoints themselves are
left enabled underneath the function's own idea of being disabled.

Call disable_usb9pfs() before reinit_completion(&usb9pfs->send),
mirroring the same sequence already used in p9_usbg_close(). Placing
it before the reinit_completion() also ensures usb_ep_disable() has
synchronously flushed any requests still queued in hardware before the
completion is reset, closing the same race that motivated resetting
the completion here in the first place.

Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Assisted-by: Claude:claude-opus-4.8
Cc: stable@vger.kernel.org
Signed-off-by: Michael Grzeschik <mgr@kernel.org>
---
 net/9p/trans_usbg.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index e3af8e1002d7..af113746d2fc 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -765,6 +765,10 @@ static void usb9pfs_disable(struct usb_function *f)
 		usb9pfs->client->status = Disconnected;
 	spin_unlock_irqrestore(&usb9pfs->lock, flags);
 	usb9pfs_clear_tx(usb9pfs);
+
+	if (usb9pfs->in_ep->enabled)
+		disable_usb9pfs(usb9pfs);
+
 	reinit_completion(&usb9pfs->send);
 }
 

-- 
2.53.0

[PATCH 5/7] net/9p/usbg: fix out_req buffer leak in disable_usb9pfs

[Michael Grzeschik]

usb9pfs->out_req is allocated with alloc_ep_req(), which also
allocates the request's data buffer (req->buf). disable_usb9pfs()
frees it with plain usb_ep_free_request(), which only frees the
usb_request structure itself and leaks req->buf on every close.

Use free_ep_req() instead, matching the allocator, as documented by
its own comment ("Requests allocated via alloc_ep_req() *must* be
freed by free_ep_req()").

Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Michael Grzeschik <mgr@kernel.org>
---
 net/9p/trans_usbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index af113746d2fc..9bcad638d827 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -284,7 +284,7 @@ static void disable_usb9pfs(struct f_usb9pfs *usb9pfs)
 	}
 
 	if (usb9pfs->out_req) {
-		usb_ep_free_request(usb9pfs->out_ep, usb9pfs->out_req);
+		free_ep_req(usb9pfs->out_ep, usb9pfs->out_req);
 		usb9pfs->out_req = NULL;
 	}
 

-- 
2.53.0

[PATCH 6/7] net/9p/usbg: remove bogus context initialization in alloc_requests

[Michael Grzeschik]

alloc_requests() initializes usb9pfs->in_req->context to point at the
struct f_usb9pfs instance itself. usb9pfs_queue_tx() later overwrites
this with the real struct p9_req_t pointer before every transmit, and
usb9pfs_tx_complete() clears it back to NULL after each completion, so
in_req->context only ever holds a valid p9_req_t once a request has
actually been queued.

usb9pfs_clear_tx(), however, can run at any time (mount close, gadget
disable) independent of whether a request was ever queued. If it runs
before the first usb9pfs_transmit(), it reads the leftover sentinel
value, type-confuses the struct f_usb9pfs pointer as a struct
p9_req_t, and both writes through it (req->t_err = -ECONNRESET) and
hands it to p9_client_cb(), which manipulates req->wq and req->refcount
at bogus offsets inside f_usb9pfs. This reliably corrupts memory or
panics whenever the transport is torn down before any 9p request has
been transmitted, e.g. mounting and immediately unmounting, or a cable
disconnect racing the very first request.

usb9pfs->out_req->context is set the same way but is never read by
this transport (usb9pfs_rx_complete() identifies the instance via
ep->driver_data instead), so it serves no purpose either.

Both endpoints' ->driver_data are already set to usb9pfs in
enable_endpoint(), which is what the completion handlers actually use
to recover the f_usb9pfs instance. Just drop the leftover
->context assignments; usb_ep_alloc_request()/alloc_ep_req() already
return zeroed requests, so in_req->context correctly starts out NULL.

Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Michael Grzeschik <mgr@kernel.org>
---
 net/9p/trans_usbg.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index 9bcad638d827..544690a5c717 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -314,10 +314,6 @@ static int alloc_requests(struct usb_composite_dev *cdev,
 	usb9pfs->in_req->complete = usb9pfs_tx_complete;
 	usb9pfs->out_req->complete = usb9pfs_rx_complete;
 
-	/* length will be set in complete routine */
-	usb9pfs->in_req->context = usb9pfs;
-	usb9pfs->out_req->context = usb9pfs;
-
 	return 0;
 
 fail_in:

-- 
2.53.0

[PATCH 7/7] net/9p/usbg: clear stale client pointer on close

[Michael Grzeschik]

From: Hyungjung Joo <jhj140711@gmail.com>

p9_usbg_close() tears down the client transport, but usb9pfs keeps
using usb9pfs->client from asynchronous TX and RX completion handlers.
A late completion can therefore dereference a client that has already
been freed during mount teardown.

Clear usb9pfs->client under usb9pfs->lock when closing the transport,
detach any pending TX request from in_req->context, and make the TX/RX
completion handlers bail out once the transport has been detached. This
keeps late completions from touching a freed or rebound p9_client.

Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Cc: stable@vger.kernel.org
Signed-off-by: Hyungjung Joo <jhj140711@gmail.com>
Signed-off-by: Michael Grzeschik <mgr@kernel.org>
---
 net/9p/trans_usbg.c | 61 +++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 16 deletions(-)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index 544690a5c717..c6306b073089 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -149,7 +149,8 @@ static void usb9pfs_tx_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	struct f_usb9pfs *usb9pfs = ep->driver_data;
 	struct usb_composite_dev *cdev = usb9pfs->function.config->cdev;
-	struct p9_req_t *p9_tx_req = req->context;
+	struct p9_client *client;
+	struct p9_req_t *p9_tx_req;
 	unsigned long flags;
 
 	/* reset zero packages */
@@ -165,18 +166,25 @@ static void usb9pfs_tx_complete(struct usb_ep *ep, struct usb_request *req)
 		ep->name, req->status, req->actual, req->length);
 
 	spin_lock_irqsave(&usb9pfs->lock, flags);
-	WRITE_ONCE(p9_tx_req->status, REQ_STATUS_SENT);
+	client = usb9pfs->client;
+	p9_tx_req = req->context;
+	req->context = NULL;
 
-	p9_req_put(usb9pfs->client, p9_tx_req);
+	if (!client || !p9_tx_req)
+		goto unlock_complete;
 
-	req->context = NULL;
+	WRITE_ONCE(p9_tx_req->status, REQ_STATUS_SENT);
 
+	p9_req_put(client, p9_tx_req);
+
+unlock_complete:
 	spin_unlock_irqrestore(&usb9pfs->lock, flags);
 
 	complete(&usb9pfs->send);
 }
 
-static struct p9_req_t *usb9pfs_rx_header(struct f_usb9pfs *usb9pfs, void *buf)
+static struct p9_req_t *usb9pfs_rx_header(struct f_usb9pfs *usb9pfs,
+					  struct p9_client *client, void *buf)
 {
 	struct p9_req_t *p9_rx_req;
 	struct p9_fcall	rc;
@@ -202,7 +210,7 @@ static struct p9_req_t *usb9pfs_rx_header(struct f_usb9pfs *usb9pfs, void *buf)
 		 "mux %p pkt: size: %d bytes tag: %d\n",
 		 usb9pfs, rc.size, rc.tag);
 
-	p9_rx_req = p9_tag_lookup(usb9pfs->client, rc.tag);
+	p9_rx_req = p9_tag_lookup(client, rc.tag);
 	if (!p9_rx_req || p9_rx_req->status != REQ_STATUS_SENT) {
 		p9_debug(P9_DEBUG_ERROR, "Unexpected packet tag %d\n", rc.tag);
 		return NULL;
@@ -212,7 +220,7 @@ static struct p9_req_t *usb9pfs_rx_header(struct f_usb9pfs *usb9pfs, void *buf)
 		p9_debug(P9_DEBUG_ERROR,
 			 "requested packet size too big: %d for tag %d with capacity %zd\n",
 			 rc.size, rc.tag, p9_rx_req->rc.capacity);
-		p9_req_put(usb9pfs->client, p9_rx_req);
+		p9_req_put(client, p9_rx_req);
 		return NULL;
 	}
 
@@ -220,7 +228,7 @@ static struct p9_req_t *usb9pfs_rx_header(struct f_usb9pfs *usb9pfs, void *buf)
 		p9_debug(P9_DEBUG_ERROR,
 			 "No recv fcall for tag %d (req %p), disconnecting!\n",
 			 rc.tag, p9_rx_req);
-		p9_req_put(usb9pfs->client, p9_rx_req);
+		p9_req_put(client, p9_rx_req);
 		return NULL;
 	}
 
@@ -231,8 +239,10 @@ static void usb9pfs_rx_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	struct f_usb9pfs *usb9pfs = ep->driver_data;
 	struct usb_composite_dev *cdev = usb9pfs->function.config->cdev;
+	struct p9_client *client;
 	struct p9_req_t *p9_rx_req;
 	unsigned int req_size = req->actual;
+	unsigned long flags;
 	int status = REQ_STATUS_RCVD;
 
 	if (req->status) {
@@ -241,9 +251,16 @@ static void usb9pfs_rx_complete(struct usb_ep *ep, struct usb_request *req)
 		return;
 	}
 
-	p9_rx_req = usb9pfs_rx_header(usb9pfs, req->buf);
-	if (!p9_rx_req)
+	spin_lock_irqsave(&usb9pfs->lock, flags);
+	client = usb9pfs->client;
+	if (!client) {
+		spin_unlock_irqrestore(&usb9pfs->lock, flags);
 		return;
+	}
+
+	p9_rx_req = usb9pfs_rx_header(usb9pfs, client, req->buf);
+	if (!p9_rx_req)
+		goto out_unlock;
 
 	if (req_size > p9_rx_req->rc.capacity) {
 		dev_err(&cdev->gadget->dev,
@@ -257,8 +274,11 @@ static void usb9pfs_rx_complete(struct usb_ep *ep, struct usb_request *req)
 
 	p9_rx_req->rc.size = req_size;
 
-	p9_client_cb(usb9pfs->client, p9_rx_req, status);
-	p9_req_put(usb9pfs->client, p9_rx_req);
+	p9_client_cb(client, p9_rx_req, status);
+	p9_req_put(client, p9_rx_req);
+
+out_unlock:
+	spin_unlock_irqrestore(&usb9pfs->lock, flags);
 
 	complete(&usb9pfs->received);
 }
@@ -412,7 +432,9 @@ static int p9_usbg_create(struct p9_client *client, struct fs_context *fc)
 		client->status = Disconnected;
 	else
 		client->status = Connected;
+	spin_lock_irq(&usb9pfs->lock);
 	usb9pfs->client = client;
+	spin_unlock_irq(&usb9pfs->lock);
 
 	client->trans_mod->maxsize = usb9pfs->buflen;
 
@@ -423,12 +445,19 @@ static int p9_usbg_create(struct p9_client *client, struct fs_context *fc)
 
 static void usb9pfs_clear_tx(struct f_usb9pfs *usb9pfs)
 {
+	struct p9_client *client;
 	struct p9_req_t *req;
+	unsigned long flags;
 
-	guard(spinlock_irqsave)(&usb9pfs->lock);
+	spin_lock_irqsave(&usb9pfs->lock, flags);
+	client = usb9pfs->client;
+	usb9pfs->client = NULL;
+	req = usb9pfs->in_req ? usb9pfs->in_req->context : NULL;
+	if (usb9pfs->in_req)
+		usb9pfs->in_req->context = NULL;
+	spin_unlock_irqrestore(&usb9pfs->lock, flags);
 
-	req = usb9pfs->in_req->context;
-	if (!req)
+	if (!req || !client)
 		return;
 
 	usb9pfs->in_req->context = NULL;
@@ -439,7 +468,7 @@ static void usb9pfs_clear_tx(struct f_usb9pfs *usb9pfs)
 	if (!req->t_err)
 		req->t_err = -ECONNRESET;
 
-	p9_client_cb(usb9pfs->client, req, REQ_STATUS_ERROR);
+	p9_client_cb(client, req, REQ_STATUS_ERROR);
 	usb9pfs->in_req->context = NULL;
 }
 

-- 
2.53.0

Re: [PATCH 0/7] usb9pfs: stable fixes

[Dominique Martinet]

Michael Grzeschik wrote on Tue, Sep 22, 2026 at 12:25:53AM +0200:
> This is a breakout series with some additional fixes of the
> first series of fixes I did sent out earlier this year.
> 
> https://lore.kernel.org/all/20260319-9pfixes-v1-0-c977a7433185@pengutronix.de/
> 
> It only addresses the stability of the driver and left out
> the interface rework. Therefor I put all changes into a new
> series beginning as v1.

Thanks!
Also thanks for picking up a patch I had missed earlier.
This didn't apply cleanly but nothing a fuzzy `patch` command couldn't
handle (unfortunately sashiko gives up too easily on the whole
series... but there's nothing controversial in here anyway so I won't
ask you to resend)

I fixed a typo in one of the messages but that aside this looks good to
me, I've picked this up and will send to -next in a bit

-- 
Dominique Martinet | Asmadeus