[PATCH 00/11] net/9p/usbg: series of fixes

[PATCH 00/11] net/9p/usbg: series of fixes

[Michael Grzeschik]

This series contains a bunch of patches to make the trans_usbg
interface more reliable. It adds some extra checks on critical
pathes and also solves the overall synchronisation of the daemon
with the gadget. The forwarder script also gained the daemon mode to
be run and recover any kind of disconnection.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
Hyungjung Joo (1):
      net/9p/usbg: clear stale client pointer on close

Michael Grzeschik (10):
      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: only rely on one completion
      net/9p/usbg: add timeout for usbg_request
      net/9p/usbg: add extra interface for status change
      tools/usb/p9_fwd: catch if claim_interface is not working
      tools/usb/p9_fwd: catch write or read errors on disconnect
      tools/usb/p9_fwd: add daemon loop
      tools/usb/p9_fwd: set new introduced alt mode 1 on interface 1

 net/9p/trans_usbg.c | 211 +++++++++++++++++++++++++++++++++++++++++-----------
 tools/usb/p9_fwd.py |  67 +++++++++++++----
 2 files changed, 218 insertions(+), 60 deletions(-)
---
base-commit: 8a30aeb0d1b4e4aaf7f7bae72f20f2ae75385ccb
change-id: 20260128-9pfixes-442c28f40622

Best regards,
-- 
Michael Grzeschik <m.grzeschik@pengutronix.de>

[PATCH 01/11] 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
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Hyungjung Joo <jhj140711@gmail.com>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 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 1ce70338999c8c712ba298efa88c69e6372ac40f..f7a94572013e7d1015d75fb5dbdde5eb81f7d7d0 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);
 }
@@ -416,7 +436,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 +449,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.47.3

[PATCH 02/11] 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 <m.grzeschik@pengutronix.de>
---
 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 f7a94572013e7d1015d75fb5dbdde5eb81f7d7d0..fb05198dc2a7d604cfad2db26a63e40e632651a2 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -495,6 +495,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.47.3

[PATCH 03/11] 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 <m.grzeschik@pengutronix.de>
---
 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 fb05198dc2a7d604cfad2db26a63e40e632651a2..6ddf6886dbadd7cdfdebb96dc767874169ccb16e 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -779,7 +779,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.47.3

[PATCH 04/11] 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 <m.grzeschik@pengutronix.de>
---
 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 6ddf6886dbadd7cdfdebb96dc767874169ccb16e..d6391db6d5d96a1609a3405646f66d82c93d35f1 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -497,6 +497,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)
@@ -786,6 +787,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.47.3

[PATCH 05/11] net/9p/usbg: only rely on one completion

[Michael Grzeschik]

The 9pfs protocol is specified that each request is answered with one
response. Since there is only one pair of tx and rx requests for the usb
stack, the stack complete callback of the tx request can be used to
queue the response.

The current approach is using an extra completion barrier to ensure that
the tx complete callback came back, before enqueuing the rx request.
This all was done in the initial 9p_usbg_request callback of the 9pfs
protocol.

Moving the ep_queue of the rx request removes one extra completion
barrier and is less complex. Other than the complete this call has to be
guarded by the spinlock.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 net/9p/trans_usbg.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index d6391db6d5d96a1609a3405646f66d82c93d35f1..472f7452794d7ac0f7b1475f0da2f4605a28e062 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -50,7 +50,6 @@ struct f_usb9pfs {
 	struct usb_ep *out_ep;
 
 	struct completion send;
-	struct completion received;
 
 	unsigned int buflen;
 
@@ -180,7 +179,7 @@ static void usb9pfs_tx_complete(struct usb_ep *ep, struct usb_request *req)
 unlock_complete:
 	spin_unlock_irqrestore(&usb9pfs->lock, flags);
 
-	complete(&usb9pfs->send);
+	usb9pfs_queue_rx(usb9pfs, usb9pfs->out_req, GFP_ATOMIC);
 }
 
 static struct p9_req_t *usb9pfs_rx_header(struct f_usb9pfs *usb9pfs,
@@ -280,7 +279,7 @@ static void usb9pfs_rx_complete(struct usb_ep *ep, struct usb_request *req)
 out_unlock:
 	spin_unlock_irqrestore(&usb9pfs->lock, flags);
 
-	complete(&usb9pfs->received);
+	complete(&usb9pfs->send);
 }
 
 static void disable_ep(struct usb_composite_dev *cdev, struct usb_ep *ep)
@@ -442,7 +441,7 @@ static int p9_usbg_create(struct p9_client *client, struct fs_context *fc)
 
 	client->trans_mod->maxsize = usb9pfs->buflen;
 
-	complete(&usb9pfs->received);
+	complete(&usb9pfs->send);
 
 	return 0;
 }
@@ -508,7 +507,7 @@ static int p9_usbg_request(struct p9_client *client, struct p9_req_t *p9_req)
 	if (client->status != Connected)
 		return -EBUSY;
 
-	ret = wait_for_completion_killable(&usb9pfs->received);
+	ret = wait_for_completion_killable(&usb9pfs->send);
 	if (ret)
 		return ret;
 
@@ -516,11 +515,8 @@ static int p9_usbg_request(struct p9_client *client, struct p9_req_t *p9_req)
 	if (ret)
 		return ret;
 
-	ret = wait_for_completion_killable(&usb9pfs->send);
-	if (ret)
-		return ret;
+	return 0;
 
-	return usb9pfs_queue_rx(usb9pfs, usb9pfs->out_req, GFP_ATOMIC);
 }
 
 static int p9_usbg_cancel(struct p9_client *client, struct p9_req_t *req)
@@ -802,7 +798,6 @@ static struct usb_function *usb9pfs_alloc(struct usb_function_instance *fi)
 	spin_lock_init(&usb9pfs->lock);
 
 	init_completion(&usb9pfs->send);
-	init_completion(&usb9pfs->received);
 
 	usb9pfs_opts = container_of(fi, struct f_usb9pfs_opts, func_inst);
 

-- 
2.47.3

[PATCH 06/11] net/9p/usbg: add timeout for usbg_request

[Michael Grzeschik]

Move to wait_for_completion_killable_timeout to ensure that
a pending mount will not hang in there forever. If the host side is not
available after some time, the pending mount will come back with
-ETIMEDOUT.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 net/9p/trans_usbg.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index 472f7452794d7ac0f7b1475f0da2f4605a28e062..c764e06ad4bf7ef7266160b06063a4a98226649c 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -502,14 +502,21 @@ static void p9_usbg_close(struct p9_client *client)
 static int p9_usbg_request(struct p9_client *client, struct p9_req_t *p9_req)
 {
 	struct f_usb9pfs *usb9pfs = client->trans;
+	struct usb_composite_dev *cdev =
+		usb9pfs->function.config->cdev;
 	int ret;
 
 	if (client->status != Connected)
 		return -EBUSY;
 
-	ret = wait_for_completion_killable(&usb9pfs->send);
-	if (ret)
+	ret = wait_for_completion_killable_timeout(&usb9pfs->send, HZ * 10);
+	if (ret < 0)
 		return ret;
+	if (!ret) {
+		dev_err(&cdev->gadget->dev,
+			"timeout while transferring 9p via usb\n");
+		return -ETIMEDOUT;
+	}
 
 	ret = usb9pfs_transmit(usb9pfs, p9_req);
 	if (ret)

-- 
2.47.3

[PATCH 07/11] net/9p/usbg: add extra interface for status change

[Michael Grzeschik]

In the current implementation of the trans_usbg the status "connected",
when all components are ready to transfer, is not well chosen.

For now the 9p_usbg_create function looks if the in_ep is enabled, if
not, it sets the 9p as disconnected. On the other side when the set_alt
function calls ep_enable it checks if usbg_create was already called and
client is set, if so it then sets the client as connected.

With the Disconnected 9pfs layer a mount would always fail. However,
after the set_alt to 0, the mount would still fail. Since it is unsure
if there is actually something running to transfer data. For this
constraint to overcome, the trans_usbg is working with an completion in
the p9_usbg_request callback.

This has many limitations, since from the usb perspective it is true
that the wire connection is triggering the initial set_alt to 0 and
therefor can tell the 9pfs protocol that the instance now is physically
connected. This still does not tell anything about the connection and
availability of the 9pfs transport that is providing the protocol over
the usb.

To solve this issue, this patch is adding an extra interface, that
actually is containing the endpoints which will transfer the data in an
separate alt mode. This is likewise to other standards as e.g. the ncm
gadget.

The 9p transport host, that will make the 9pfs protocol available over
the two endpoints will then have to call set_alt(1) on this data
interface.

With this approach the gadget has the proper status change and
knows when to expect to receive data via the endpoints.

- don't allow set_alt when client is not ready

Avoid letting the endpoints to be enabled, when the client is not yet
prepared.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 net/9p/trans_usbg.c | 119 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 101 insertions(+), 18 deletions(-)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index c764e06ad4bf7ef7266160b06063a4a98226649c..5c954749ddba23d03eed85b9f9f94add9c5f9bf4 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -40,6 +40,7 @@
 struct f_usb9pfs {
 	struct p9_client *client;
 
+	u8 ctrl_id, data_id;
 	/* 9p request lock for en/dequeue */
 	spinlock_t lock;
 
@@ -85,6 +86,12 @@ struct f_usb9pfs_dev {
 	struct list_head usb9pfs_instance;
 };
 
+static inline struct f_usb9pfs_dev *usb9pfs_to_usb9pfs_dev(struct f_usb9pfs *usb9pfs)
+{
+	return container_of(usb9pfs->function.fi,
+			    struct f_usb9pfs_opts, func_inst)->dev;
+}
+
 static DEFINE_MUTEX(usb9pfs_lock);
 static struct list_head usbg_instance_list;
 
@@ -364,9 +371,11 @@ static int enable_endpoint(struct usb_composite_dev *cdev,
 }
 
 static int
-enable_usb9pfs(struct usb_composite_dev *cdev, struct f_usb9pfs *usb9pfs)
+enable_usb9pfs(struct f_usb9pfs *usb9pfs)
 {
-	struct p9_client *client;
+	struct usb_composite_dev *cdev =
+		usb9pfs->function.config->cdev;
+
 	int ret = 0;
 
 	ret = enable_endpoint(cdev, usb9pfs, usb9pfs->in_ep);
@@ -381,10 +390,6 @@ enable_usb9pfs(struct usb_composite_dev *cdev, struct f_usb9pfs *usb9pfs)
 	if (ret)
 		goto disable_out;
 
-	client = usb9pfs->client;
-	if (client)
-		client->status = Connected;
-
 	dev_dbg(&cdev->gadget->dev, "%s enabled\n", usb9pfs->function.name);
 	return 0;
 
@@ -431,17 +436,13 @@ static int p9_usbg_create(struct p9_client *client, struct fs_context *fc)
 		return -EINVAL;
 
 	client->trans = (void *)usb9pfs;
-	if (!usb9pfs->in_req)
-		client->status = Disconnected;
-	else
-		client->status = Connected;
+	client->status = Connected;
 	spin_lock_irq(&usb9pfs->lock);
 	usb9pfs->client = client;
 	spin_unlock_irq(&usb9pfs->lock);
 
 	client->trans_mod->maxsize = usb9pfs->buflen;
 
-	complete(&usb9pfs->send);
 
 	return 0;
 }
@@ -563,7 +564,7 @@ static struct usb_interface_descriptor usb9pfs_intf = {
 	.bLength =		sizeof(usb9pfs_intf),
 	.bDescriptorType =	USB_DT_INTERFACE,
 
-	.bNumEndpoints =	2,
+	.bNumEndpoints =	0,
 	.bInterfaceClass =	USB_CLASS_VENDOR_SPEC,
 	.bInterfaceSubClass =	USB_SUBCLASS_VENDOR_SPEC,
 	.bInterfaceProtocol =   USB_PROTOCOL_9PFS,
@@ -571,6 +572,36 @@ static struct usb_interface_descriptor usb9pfs_intf = {
 	/* .iInterface = DYNAMIC */
 };
 
+/* the default data interface has no endpoints ... */
+
+static struct usb_interface_descriptor usb9pfs_data_nop_intf = {
+	.bLength =		sizeof(usb9pfs_data_nop_intf),
+	.bDescriptorType =	USB_DT_INTERFACE,
+
+	.bInterfaceNumber =	1,
+	.bAlternateSetting =	0,
+	.bNumEndpoints =	0,
+	.bInterfaceClass =	USB_CLASS_VENDOR_SPEC,
+	.bInterfaceSubClass =	USB_SUBCLASS_VENDOR_SPEC,
+	.bInterfaceProtocol =   USB_PROTOCOL_9PFS,
+	/* .iInterface = DYNAMIC */
+};
+
+/* ... but the "real" data interface has two bulk endpoints */
+
+static struct usb_interface_descriptor usb9pfs_data_intf = {
+	.bLength =		sizeof(usb9pfs_data_intf),
+	.bDescriptorType =	USB_DT_INTERFACE,
+
+	.bInterfaceNumber =	1,
+	.bAlternateSetting =	1,
+	.bNumEndpoints =	2,
+	.bInterfaceClass =	USB_CLASS_VENDOR_SPEC,
+	.bInterfaceSubClass =	USB_SUBCLASS_VENDOR_SPEC,
+	.bInterfaceProtocol =   USB_PROTOCOL_9PFS,
+	/* .iInterface = DYNAMIC */
+};
+
 /* full speed support: */
 
 static struct usb_endpoint_descriptor fs_usb9pfs_source_desc = {
@@ -591,6 +622,8 @@ static struct usb_endpoint_descriptor fs_usb9pfs_sink_desc = {
 
 static struct usb_descriptor_header *fs_usb9pfs_descs[] = {
 	(struct usb_descriptor_header *)&usb9pfs_intf,
+	(struct usb_descriptor_header *)&usb9pfs_data_nop_intf,
+	(struct usb_descriptor_header *)&usb9pfs_data_intf,
 	(struct usb_descriptor_header *)&fs_usb9pfs_sink_desc,
 	(struct usb_descriptor_header *)&fs_usb9pfs_source_desc,
 	NULL,
@@ -616,6 +649,8 @@ static struct usb_endpoint_descriptor hs_usb9pfs_sink_desc = {
 
 static struct usb_descriptor_header *hs_usb9pfs_descs[] = {
 	(struct usb_descriptor_header *)&usb9pfs_intf,
+	(struct usb_descriptor_header *)&usb9pfs_data_nop_intf,
+	(struct usb_descriptor_header *)&usb9pfs_data_intf,
 	(struct usb_descriptor_header *)&hs_usb9pfs_source_desc,
 	(struct usb_descriptor_header *)&hs_usb9pfs_sink_desc,
 	NULL,
@@ -657,6 +692,8 @@ static struct usb_ss_ep_comp_descriptor ss_usb9pfs_sink_comp_desc = {
 
 static struct usb_descriptor_header *ss_usb9pfs_descs[] = {
 	(struct usb_descriptor_header *)&usb9pfs_intf,
+	(struct usb_descriptor_header *)&usb9pfs_data_nop_intf,
+	(struct usb_descriptor_header *)&usb9pfs_data_intf,
 	(struct usb_descriptor_header *)&ss_usb9pfs_source_desc,
 	(struct usb_descriptor_header *)&ss_usb9pfs_source_comp_desc,
 	(struct usb_descriptor_header *)&ss_usb9pfs_sink_desc,
@@ -666,7 +703,9 @@ static struct usb_descriptor_header *ss_usb9pfs_descs[] = {
 
 /* function-specific strings: */
 static struct usb_string strings_usb9pfs[] = {
-	[0].s = "usb9pfs input to output",
+	[0].s = "usb9pfs control",
+	[1].s = "usb9pfs nop",
+	[2].s = "usb9pfs data",
 	{  }			/* end of list */
 };
 
@@ -688,20 +727,34 @@ static int usb9pfs_func_bind(struct usb_configuration *c,
 	struct f_usb9pfs *usb9pfs = func_to_usb9pfs(f);
 	struct f_usb9pfs_opts *opts;
 	struct usb_composite_dev *cdev = c->cdev;
+	struct usb_string *us;
 	int ret;
 	int id;
 
+	us = usb_gstrings_attach(cdev, usb9pfs_strings,
+				 ARRAY_SIZE(strings_usb9pfs));
+	if (IS_ERR(us))
+		return PTR_ERR(us);
+
+	usb9pfs_intf.iInterface = us[0].id;
+	usb9pfs_data_nop_intf.iInterface = us[1].id;
+	usb9pfs_data_intf.iInterface = us[2].id;
+
 	/* allocate interface ID(s) */
 	id = usb_interface_id(c, f);
 	if (id < 0)
 		return id;
+
+	usb9pfs->ctrl_id = id;
 	usb9pfs_intf.bInterfaceNumber = id;
 
-	id = usb_string_id(cdev);
+	id = usb_interface_id(c, f);
 	if (id < 0)
 		return id;
-	strings_usb9pfs[0].id = id;
-	usb9pfs_intf.iInterface = id;
+
+	usb9pfs->data_id = id;
+	usb9pfs_data_nop_intf.bInterfaceNumber = id;
+	usb9pfs_data_intf.bInterfaceNumber = id;
 
 	/* allocate endpoints */
 	usb9pfs->in_ep = usb_ep_autoconfig(cdev->gadget,
@@ -775,9 +828,38 @@ static int usb9pfs_set_alt(struct usb_function *f,
 			   unsigned int intf, unsigned int alt)
 {
 	struct f_usb9pfs *usb9pfs = func_to_usb9pfs(f);
-	struct usb_composite_dev *cdev = f->config->cdev;
+	int ret;
+
+	if (!alt) {
+		if (intf == usb9pfs->data_id &&
+		    usb9pfs->in_ep->enabled)
+			disable_usb9pfs(usb9pfs);
+		return 0;
+	}
+
+	if (!usb9pfs->client || !usb9pfs_to_usb9pfs_dev(usb9pfs)->inuse)
+		return -EINVAL;
+
+	ret = enable_usb9pfs(usb9pfs);
+	if (ret)
+		return ret;
+
+	complete(&usb9pfs->send);
+
+	return 0;
+}
+
+/*
+ * Because the data interface supports multiple altsettings,
+ * this 9pfs function *MUST* implement a get_alt() method.
+ */
+static int usb9pfs_get_alt(struct usb_function *f, unsigned int intf)
+{
+	struct f_usb9pfs *usb9pfs = func_to_usb9pfs(f);
 
-	return enable_usb9pfs(cdev, usb9pfs);
+	if (intf == usb9pfs->ctrl_id)
+		return 0;
+	return usb9pfs->in_ep->enabled ? 1 : 0;
 }
 
 static void usb9pfs_disable(struct usb_function *f)
@@ -818,6 +900,7 @@ static struct usb_function *usb9pfs_alloc(struct usb_function_instance *fi)
 	usb9pfs->function.bind = usb9pfs_func_bind;
 	usb9pfs->function.unbind = usb9pfs_func_unbind;
 	usb9pfs->function.set_alt = usb9pfs_set_alt;
+	usb9pfs->function.get_alt = usb9pfs_get_alt;
 	usb9pfs->function.disable = usb9pfs_disable;
 	usb9pfs->function.strings = usb9pfs_strings;
 

-- 
2.47.3

[PATCH 08/11] tools/usb/p9_fwd: catch if claim_interface is not working

[Michael Grzeschik]

It is possible that some other tool was already claiming the interface.
Bail out gracefully if this happens.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 tools/usb/p9_fwd.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/usb/p9_fwd.py b/tools/usb/p9_fwd.py
index 12c76cbb046b7277bdd0ec39b663a041c9a1fc89..6a0104935ee995ec4159e27e1cfb7d93db422a33 100755
--- a/tools/usb/p9_fwd.py
+++ b/tools/usb/p9_fwd.py
@@ -79,7 +79,13 @@ class Forwarder:
             raise ValueError("Interface not found")
 
         logging.info(f"claiming interface:\n{usb9pfs}")
-        usb.util.claim_interface(dev, usb9pfs.bInterfaceNumber)
+        try:
+            usb.util.claim_interface(dev, usb9pfs.bInterfaceNumber)
+        except usb.core.USBError as e:
+            if e.errno == errno.EBUSY:
+                logging.debug("old connection was hanging", repr(e))
+                raise ValueError("disconnected")
+
         ep_out = usb.util.find_descriptor(
             usb9pfs,
             custom_match=lambda e: usb.util.endpoint_direction(e.bEndpointAddress) == usb.util.ENDPOINT_OUT,

-- 
2.47.3

[PATCH 09/11] tools/usb/p9_fwd: catch write or read errors on disconnect

[Michael Grzeschik]

When unplugging the gadget or the gadget is not available
since the controller is reset, running transfers will return
with errno ENODEV. We catch this issues and report the problem
gracefully.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 tools/usb/p9_fwd.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tools/usb/p9_fwd.py b/tools/usb/p9_fwd.py
index 6a0104935ee995ec4159e27e1cfb7d93db422a33..c2c2afbf45df68376907ea2ae8c022614e4508f7 100755
--- a/tools/usb/p9_fwd.py
+++ b/tools/usb/p9_fwd.py
@@ -123,6 +123,9 @@ class Forwarder:
                     logging.debug("c2s: reading failed with %s, retrying", repr(e))
                     time.sleep(0.5)
                     continue
+                elif e.errno == errno.ENODEV:
+                    logging.debug("c2s: reading failed with %s", repr(e))
+                    raise ValueError("disconnected")
                 logging.error("c2s: reading failed with %s, aborting", repr(e))
                 raise
         size = struct.unpack("<I", data[:4])[0]
@@ -145,9 +148,14 @@ class Forwarder:
         logging.log(logging.TRACE, "s2c: writing")
         self._log_hexdump(data)
         while data:
-            written = self.ep_out.write(data)
-            assert written > 0
-            data = data[written:]
+            try:
+                written = self.ep_out.write(data)
+                assert written > 0
+                data = data[written:]
+            except usb.core.USBError as e:
+                if e.errno == errno.EIO or e.errno == errno.ENODEV:
+                   raise ValueError("disconnected")
+                raise
         if size % self.ep_out.wMaxPacketSize == 0:
             logging.log(logging.TRACE, "sending zero length packet")
             self.ep_out.write(b"")

-- 
2.47.3

[PATCH 10/11] tools/usb/p9_fwd: add daemon loop

[Michael Grzeschik]

When the tool is started always catch connection and transfer
errors and recover. We also log in one line, how long the
Forwarder was not able to connect.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 tools/usb/p9_fwd.py | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/tools/usb/p9_fwd.py b/tools/usb/p9_fwd.py
index c2c2afbf45df68376907ea2ae8c022614e4508f7..1b479c931df9bfb0f7e7f2cf5a612764408f0828 100755
--- a/tools/usb/p9_fwd.py
+++ b/tools/usb/p9_fwd.py
@@ -7,6 +7,7 @@ import logging
 import socket
 import struct
 import time
+import sys
 
 import usb.core
 import usb.util
@@ -59,7 +60,7 @@ class Forwarder:
         if dev is None:
             raise ValueError("Device not found")
 
-        logging.info(f"found device: {dev.bus}/{dev.address} located at {path_from_usb_dev(dev)}")
+        logging.info(f"\nfound device: {dev.bus}/{dev.address} located at {path_from_usb_dev(dev)}")
 
         # dev.set_configuration() is not necessary since g_multi has only one
         usb9pfs = None
@@ -201,16 +202,28 @@ def list_usb(args):
 def connect(args):
     vid, pid = [int(x, 16) for x in args.id.split(":", 1)]
 
-    f = Forwarder(server=(args.server, args.port), vid=vid, pid=pid, path=args.path)
-
-    try:
-        while True:
-            f.c2s()
-            f.s2c()
-            f.log_stats_interval()
-    finally:
-        f.log_stats()
+    i = 0
+    while True:
+        try:
+            f = Forwarder(server=(args.server, args.port), vid=vid, pid=pid, path=args.path)
+        except ValueError as ve:
+            time.sleep(1)
+            print(f"\rdevice not found since {i} seconds, retrying",end="")
+            sys.stdout.flush()
+            i = i + 1
+            continue
 
+        try:
+            while True:
+                f.c2s()
+                f.s2c()
+                f.log_stats_interval()
+        except ValueError as ve:
+            logging.info("disconnected, retrying")
+            continue
+        finally:
+            i = 0
+            f.log_stats()
 
 def main():
     parser = argparse.ArgumentParser(

-- 
2.47.3

[PATCH 11/11] tools/usb/p9_fwd: set new introduced alt mode 1 on interface 1

[Michael Grzeschik]

Check for the right interface with the two endpoints and
enable the altmode 1 to tell the gadget it can mount now.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 tools/usb/p9_fwd.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/usb/p9_fwd.py b/tools/usb/p9_fwd.py
index 1b479c931df9bfb0f7e7f2cf5a612764408f0828..32b138976e0da0f395d25036fce8fd7faffdadad 100755
--- a/tools/usb/p9_fwd.py
+++ b/tools/usb/p9_fwd.py
@@ -74,7 +74,7 @@ class Forwarder:
                 if dev.is_kernel_driver_active(intf.bInterfaceNumber):
                     dev.detach_kernel_driver(intf.bInterfaceNumber)
 
-            if intf.bInterfaceClass == 0xFF and intf.bInterfaceSubClass == 0xFF and intf.bInterfaceProtocol == 0x09:
+            if intf.bInterfaceClass == 0xFF and intf.bInterfaceSubClass == 0xFF and intf.bInterfaceProtocol == 0x09 and intf.bNumEndpoints == 2:
                 usb9pfs = intf
         if usb9pfs is None:
             raise ValueError("Interface not found")
@@ -103,6 +103,14 @@ class Forwarder:
         self.ep_in = ep_in
         self.dev = dev
 
+        logging.info(f"setting alt mode 1 interface:\n{usb9pfs}")
+        try:
+            self.dev.set_interface_altsetting(usb9pfs.bInterfaceNumber, 1)
+        except usb.core.USBError as e:
+            if e.errno == None:
+                logging.debug("could not switch to alt 1 retry", repr(e))
+                raise ValueError("mount is not ready yet")
+
         # create and connect socket
         self.s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         self.s.connect(server)

-- 
2.47.3

Re: [PATCH 00/11] net/9p/usbg: series of fixes

[Dominique Martinet]

Michael Grzeschik wrote on Thu, Mar 19, 2026 at 10:35:57AM +0100:
> This series contains a bunch of patches to make the trans_usbg
> interface more reliable. It adds some extra checks on critical
> pathes and also solves the overall synchronisation of the daemon
> with the gadget. The forwarder script also gained the daemon mode to
> be run and recover any kind of disconnection.

Thanks for the patches

I don't have much time to review right now, but I'll try to take a
look... I still haven't gotten around to figure out how to make this
work with qemu or taken the time to set it up on $work boards two years
later so I'll have to trust you on actual tests, but it can't hurt to
review.

While I'm buying time here, I'm not one to praise our new robot
overlords but since it made the news I had a look and noticed
sashiko.dev ran on the patches:
https://sashiko.dev/#/patchset/20260319-9pfixes-v1-0-c977a7433185%40pengutronix.de

From a quick look of the above there are a lot of questions raised and I
don't think it's necessarily worth the time to answer all of them, but
if you have time to skim through them and pick up anything interesting
please say so
(as far as I'm concerned the usb setup is intended for a mostly trusted
setup in the first place... But then again this patch series started
with a security concern, so where do we start considering races could be
an issue?)


Thanks,
-- 
Dominique Martinet | Asmadeus