* [PATCH] net: 9p: usbg: clear stale client pointer on close
@ 2026-03-13 17:16 Hyungjung Joo
2026-03-13 18:06 ` Michael Grzeschik
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Hyungjung Joo @ 2026-03-13 17:16 UTC (permalink / raw)
To: ericvh, lucho, asmadeus
Cc: linux_oss, m.grzeschik, gregkh, v9fs, linux-usb, linux-kernel,
stable, Hyungjung Joo
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
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Hyungjung Joo <jhj140711@gmail.com>
---
net/9p/trans_usbg.c | 63 +++++++++++++++++++++++++++++++++------------
1 file changed, 47 insertions(+), 16 deletions(-)
diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index 1ce70338999c..3c2aa1943f93 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,27 @@ 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) {
+ spin_unlock_irqrestore(&usb9pfs->lock, flags);
+ complete(&usb9pfs->send);
+ return;
+ }
- req->context = NULL;
+ WRITE_ONCE(p9_tx_req->status, REQ_STATUS_SENT);
+
+ p9_req_put(client, p9_tx_req);
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 +212,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 +222,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 +230,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 +241,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 +253,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 +276,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);
}
@@ -416,7 +438,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;
@@ -427,18 +451,25 @@ 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;
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);
}
static void p9_usbg_close(struct p9_client *client)
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] net: 9p: usbg: clear stale client pointer on close 2026-03-13 17:16 [PATCH] net: 9p: usbg: clear stale client pointer on close Hyungjung Joo @ 2026-03-13 18:06 ` Michael Grzeschik 2026-03-14 6:01 ` Dominique Martinet ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Michael Grzeschik @ 2026-03-13 18:06 UTC (permalink / raw) To: Hyungjung Joo Cc: ericvh, lucho, asmadeus, linux_oss, gregkh, v9fs, linux-usb, linux-kernel, stable [-- Attachment #1: Type: text/plain, Size: 1313 bytes --] Hi On Sat, Mar 14, 2026 at 02:16:59AM +0900, Hyungjung Joo wrote: >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 >Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >Signed-off-by: Hyungjung Joo <jhj140711@gmail.com> I was just preparing a similar change in an series of patches. Let me come back with a test of your patch and the comparison to my changes. Thanks! Michael -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: 9p: usbg: clear stale client pointer on close 2026-03-13 17:16 [PATCH] net: 9p: usbg: clear stale client pointer on close Hyungjung Joo 2026-03-13 18:06 ` Michael Grzeschik @ 2026-03-14 6:01 ` Dominique Martinet 2026-03-14 7:24 ` Hyungjung Joo 2026-03-18 14:33 ` Michael Grzeschik 2026-03-18 23:24 ` Michael Grzeschik 3 siblings, 1 reply; 11+ messages in thread From: Dominique Martinet @ 2026-03-14 6:01 UTC (permalink / raw) To: Hyungjung Joo Cc: ericvh, lucho, linux_oss, m.grzeschik, gregkh, v9fs, linux-usb, linux-kernel, stable Hyungjung Joo wrote on Sat, Mar 14, 2026 at 02:16:59AM +0900: > 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. Just to make sure the problem is the usb9pfs struct being freed, not the p9_client itself which is still alive after the usb device is gone (until umount)? I'm surprised free_func isn't called after unbind, which should stop the queues (through disable_usb9pfs)? or are the ep being disabled not enough to ensure the callbacks are not in use? (e.g. disabling prevents further calls, but doesn't wait for currently running/queued requests?) (Also, thanks Michael for looking -- I'll let you do a first review before looking deeper) -- Dominique ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: 9p: usbg: clear stale client pointer on close 2026-03-14 6:01 ` Dominique Martinet @ 2026-03-14 7:24 ` Hyungjung Joo 2026-03-14 12:30 ` Dominique Martinet 0 siblings, 1 reply; 11+ messages in thread From: Hyungjung Joo @ 2026-03-14 7:24 UTC (permalink / raw) To: Dominique Martinet Cc: ericvh, lucho, linux_oss, m.grzeschik, gregkh, v9fs, linux-usb, linux-kernel, stable Thanks for the careful review. > Just to make sure the problem is the usb9pfs struct being freed, not the > p9_client itself which is still alive after the usb device is gone > (until umount)? The issue I am addressing is the stale use of the usb9pfs->client association after the transport has been closed. I am not relying on early free of struct f_usb9pfs for this bug. The lifetime mismatch here is that struct f_usb9pfs belongs to the gadget function and can be reused across mount sessions, while struct p9_client is per-mount. On the close path, p9_usbg_close() did not detach usb9pfs->client, so late TX/RX completions could still follow that pointer after close, including into a later remount that rebinds it. > I'm surprised free_func isn't called after unbind, which should stop the > queues (through disable_usb9pfs)? > or are the ep being disabled not enough to ensure the callbacks are not > in use? (e.g. disabling prevents further calls, but doesn't wait for > currently running/queued requests?) My understanding is that the unbind/free_func path is different from the close path at issue here. This patch is not trying to change or rely on gadget teardown ordering; it addresses the close-side race where usb9pfs->client remained attached and the completion paths still dereferenced it. That is why the patch: - clears usb9pfs->client under usb9pfs->lock on close, - detaches any pending TX request from in_req->context, and - makes TX/RX completions bail out once the transport has been detached. So the intent is to prevent late completions from using a stale or rebound client association after close, rather than to claim an early free of the gadget object itself. Thanks, 2026년 3월 14일 (토) PM 3:01, Dominique Martinet <asmadeus@codewreck.org>님이 작성: > > Hyungjung Joo wrote on Sat, Mar 14, 2026 at 02:16:59AM +0900: > > 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. > > Just to make sure the problem is the usb9pfs struct being freed, not the > p9_client itself which is still alive after the usb device is gone > (until umount)? > > I'm surprised free_func isn't called after unbind, which should stop the > queues (through disable_usb9pfs)? > or are the ep being disabled not enough to ensure the callbacks are not > in use? (e.g. disabling prevents further calls, but doesn't wait for > currently running/queued requests?) > > > (Also, thanks Michael for looking -- I'll let you do a first review > before looking deeper) > -- > Dominique ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: 9p: usbg: clear stale client pointer on close 2026-03-14 7:24 ` Hyungjung Joo @ 2026-03-14 12:30 ` Dominique Martinet 0 siblings, 0 replies; 11+ messages in thread From: Dominique Martinet @ 2026-03-14 12:30 UTC (permalink / raw) To: Hyungjung Joo Cc: ericvh, lucho, linux_oss, m.grzeschik, gregkh, v9fs, linux-usb, linux-kernel, stable Hyungjung Joo wrote on Sat, Mar 14, 2026 at 04:24:19PM +0900: > Thanks for the careful review. > > > Just to make sure the problem is the usb9pfs struct being freed, not the > > p9_client itself which is still alive after the usb device is gone > > (until umount)? > > The issue I am addressing is the stale use of the usb9pfs->client > association after the transport has been closed. I am not relying on > early free of struct f_usb9pfs for this bug. > > The lifetime mismatch here is that struct f_usb9pfs belongs to the > gadget function and can be reused across mount sessions, while > struct p9_client is per-mount. On the close path, p9_usbg_close() did > not detach usb9pfs->client, so late TX/RX completions could still > follow that pointer after close, including into a later remount that > rebinds it. Ah, you did write p9_usbg_close() in the commit message and I skipped over it too fast... I also misread your locking, I thought you were just getting a client pointer under lock, but you're also using it only within the lock (so there is no benefit from using a local variable as far as locking goes, but releasing the lock early would be invalid as the client itself is freed immediately after close...) From a performance point of view I think making close() stop the ep and wait until any in flight calls would be better than taking the lock all the time, but I'm not sure how visible that'd actually be on real workloads... close() can take as long as you want, it's perfectly fine to sleep there until the usb side has finished flushing if you want. I'm curious what approach Michael picked? > > I'm surprised free_func isn't called after unbind, which should stop the > > queues (through disable_usb9pfs)? > > or are the ep being disabled not enough to ensure the callbacks are not > > in use? (e.g. disabling prevents further calls, but doesn't wait for > > currently running/queued requests?) > > My understanding is that the unbind/free_func path is different from the > close path at issue here. This patch is not trying to change or rely on > gadget teardown ordering; it addresses the close-side race where > usb9pfs->client remained attached and the completion paths still > dereferenced it. Yes, transport close corresponds to a umount call, and is unrelated to usb lifetime; I was thinking of the other way around (usb device disappearing while the mount is still alive) > That is why the patch: > - clears usb9pfs->client under usb9pfs->lock on close, > - detaches any pending TX request from in_req->context, and > - makes TX/RX completions bail out once the transport has been detached. > > So the intent is to prevent late completions from using a stale or > rebound client association after close, rather than to claim an early > free of the gadget object itself. Thank you for the details (and the patch!); I'll have a deeper look after hearing back from Michael -- Dominique Martinet | Asmadeus ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: 9p: usbg: clear stale client pointer on close 2026-03-13 17:16 [PATCH] net: 9p: usbg: clear stale client pointer on close Hyungjung Joo 2026-03-13 18:06 ` Michael Grzeschik 2026-03-14 6:01 ` Dominique Martinet @ 2026-03-18 14:33 ` Michael Grzeschik 2026-03-18 14:49 ` Hyungjung Joo 2026-03-18 23:24 ` Michael Grzeschik 3 siblings, 1 reply; 11+ messages in thread From: Michael Grzeschik @ 2026-03-18 14:33 UTC (permalink / raw) To: Hyungjung Joo Cc: ericvh, lucho, asmadeus, linux_oss, gregkh, v9fs, linux-usb, linux-kernel, stable [-- Attachment #1: Type: text/plain, Size: 7303 bytes --] Hi I have some review feedback. On Sat, Mar 14, 2026 at 02:16:59AM +0900, Hyungjung Joo wrote: >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 >Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >Signed-off-by: Hyungjung Joo <jhj140711@gmail.com> >--- > net/9p/trans_usbg.c | 63 +++++++++++++++++++++++++++++++++------------ > 1 file changed, 47 insertions(+), 16 deletions(-) > >diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c >index 1ce70338999c..3c2aa1943f93 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,27 @@ 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) { Just goto unlock_complete; >+ spin_unlock_irqrestore(&usb9pfs->lock, flags); >+ complete(&usb9pfs->send); >+ return; >+ } > >- 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) I like this change but not in this patch. Since with your patch, rx_header will be called under locking context, this is not really a necessary change. I would keep it in another patch, though and move it before your change. > { > struct p9_req_t *p9_rx_req; > struct p9_fcall rc; >@@ -202,7 +212,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 +222,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 +230,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 +241,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 +253,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 +276,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); > } >@@ -416,7 +438,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; > >@@ -427,18 +451,25 @@ 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); Same applies here, it would be nice to seperate the changes of using the extra variable to a second patch. > >- req = usb9pfs->in_req->context; >- if (!req) >+ if (!req || !client) > return; > > 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); > } > > static void p9_usbg_close(struct p9_client *client) >-- >2.34.1 I tested your patch and like the changes, however I have some extra patches on top. Would you mind if I take your changes and include my suggestions. I would still keep you as author though. These changes would then be included in my series I will send today. Regards, Michael -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: 9p: usbg: clear stale client pointer on close 2026-03-18 14:33 ` Michael Grzeschik @ 2026-03-18 14:49 ` Hyungjung Joo 0 siblings, 0 replies; 11+ messages in thread From: Hyungjung Joo @ 2026-03-18 14:49 UTC (permalink / raw) To: Michael Grzeschik Cc: ericvh, lucho, asmadeus, linux_oss, gregkh, v9fs, linux-usb, linux-kernel, stable Hi, Thank you for the feedback 2026년 3월 18일 (수) PM 11:33, Michael Grzeschik <mgr@pengutronix.de>님이 작성: > I tested your patch and like the changes, however I have some extra > patches on top. Would you mind if I take your changes and include my > suggestions. I would still keep you as author though. These changes would > then be included in my series I will send today. That sounds good to me. Feel free to take my changes and include your suggestions and thanks for keeping me as the author. Best regards, Hyungjung Joo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: 9p: usbg: clear stale client pointer on close 2026-03-13 17:16 [PATCH] net: 9p: usbg: clear stale client pointer on close Hyungjung Joo ` (2 preceding siblings ...) 2026-03-18 14:33 ` Michael Grzeschik @ 2026-03-18 23:24 ` Michael Grzeschik 2026-03-19 0:30 ` Dominique Martinet 3 siblings, 1 reply; 11+ messages in thread From: Michael Grzeschik @ 2026-03-18 23:24 UTC (permalink / raw) To: Hyungjung Joo Cc: ericvh, lucho, asmadeus, linux_oss, gregkh, v9fs, linux-usb, linux-kernel, stable [-- Attachment #1: Type: text/plain, Size: 1422 bytes --] Hi On Sat, Mar 14, 2026 at 02:16:59AM +0900, Hyungjung Joo wrote: >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 >Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> I wonder where greg did this review? Was this in another thread? I will have to drop this anyway, when splitting it appart. >Signed-off-by: Hyungjung Joo <jhj140711@gmail.com> >--- > net/9p/trans_usbg.c | 63 +++++++++++++++++++++++++++++++++------------ > 1 file changed, 47 insertions(+), 16 deletions(-) > ... -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: 9p: usbg: clear stale client pointer on close 2026-03-18 23:24 ` Michael Grzeschik @ 2026-03-19 0:30 ` Dominique Martinet 2026-03-19 9:16 ` Michael Grzeschik 0 siblings, 1 reply; 11+ messages in thread From: Dominique Martinet @ 2026-03-19 0:30 UTC (permalink / raw) To: Michael Grzeschik Cc: Hyungjung Joo, ericvh, lucho, linux_oss, gregkh, v9fs, linux-usb, linux-kernel, stable Michael Grzeschik wrote on Thu, Mar 19, 2026 at 12:24:51AM +0100: > On Sat, Mar 14, 2026 at 02:16:59AM +0900, Hyungjung Joo wrote: > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > I wonder where greg did this review? Was this in another thread? Yes, it was when Hyungjung Joo sent this to security@, so the review was not public -- Dominique Martinet | Asmadeus ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: 9p: usbg: clear stale client pointer on close 2026-03-19 0:30 ` Dominique Martinet @ 2026-03-19 9:16 ` Michael Grzeschik 2026-03-19 9:22 ` Greg KH 0 siblings, 1 reply; 11+ messages in thread From: Michael Grzeschik @ 2026-03-19 9:16 UTC (permalink / raw) To: Dominique Martinet Cc: Hyungjung Joo, ericvh, lucho, linux_oss, gregkh, v9fs, linux-usb, linux-kernel, stable [-- Attachment #1: Type: text/plain, Size: 958 bytes --] On Thu, Mar 19, 2026 at 09:30:36AM +0900, Dominique Martinet wrote: >Michael Grzeschik wrote on Thu, Mar 19, 2026 at 12:24:51AM +0100: >> On Sat, Mar 14, 2026 at 02:16:59AM +0900, Hyungjung Joo wrote: >> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> >> I wonder where greg did this review? Was this in another thread? > >Yes, it was when Hyungjung Joo sent this to security@, so the review was >not public Since the patch was already reviewed by greg, I decided to keep it as much as it is and include it in one in my series. This way I can keep the Reviewed-by and the Fixes tag for stable. Regards, Michael -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: 9p: usbg: clear stale client pointer on close 2026-03-19 9:16 ` Michael Grzeschik @ 2026-03-19 9:22 ` Greg KH 0 siblings, 0 replies; 11+ messages in thread From: Greg KH @ 2026-03-19 9:22 UTC (permalink / raw) To: Michael Grzeschik Cc: Dominique Martinet, Hyungjung Joo, ericvh, lucho, linux_oss, v9fs, linux-usb, linux-kernel, stable On Thu, Mar 19, 2026 at 10:16:05AM +0100, Michael Grzeschik wrote: > On Thu, Mar 19, 2026 at 09:30:36AM +0900, Dominique Martinet wrote: > > Michael Grzeschik wrote on Thu, Mar 19, 2026 at 12:24:51AM +0100: > > > On Sat, Mar 14, 2026 at 02:16:59AM +0900, Hyungjung Joo wrote: > > > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > > > I wonder where greg did this review? Was this in another thread? > > > > Yes, it was when Hyungjung Joo sent this to security@, so the review was > > not public > > Since the patch was already reviewed by greg, I decided to keep it > as much as it is and include it in one in my series. This way I can keep > the Reviewed-by and the Fixes tag for stable. Hey, I just did a cursory review, feel free to drop mine if the patch has changed at all, doesn't bother me in the least. thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-03-19 9:22 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-13 17:16 [PATCH] net: 9p: usbg: clear stale client pointer on close Hyungjung Joo 2026-03-13 18:06 ` Michael Grzeschik 2026-03-14 6:01 ` Dominique Martinet 2026-03-14 7:24 ` Hyungjung Joo 2026-03-14 12:30 ` Dominique Martinet 2026-03-18 14:33 ` Michael Grzeschik 2026-03-18 14:49 ` Hyungjung Joo 2026-03-18 23:24 ` Michael Grzeschik 2026-03-19 0:30 ` Dominique Martinet 2026-03-19 9:16 ` Michael Grzeschik 2026-03-19 9:22 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox