Re: [PATCH] net: 9p: usbg: clear stale client pointer on close
Hyungjung Joo <[email protected]>
| Newsgroups | dev.linux.lists.v9fs,org.kernel.vger.linux-kernel,org.kernel.vger.linux-usb,org.kernel.vger.stable |
|---|---|
| Message-ID | <CAP_j_b9i2n+ZUriWceZJgAKz_MeGMFkRiOz7yCYE4as4T6auVQ@mail.gmail.com> |
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 <[email protected]>님이 작성: > > 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