* [PATCH] net/9p: Fix buffer overflow in USB transport layer
@ 2025-06-16 13:25 Yuhao Jiang
2025-06-16 23:00 ` asmadeus
0 siblings, 1 reply; 6+ messages in thread
From: Yuhao Jiang @ 2025-06-16 13:25 UTC (permalink / raw)
To: ericvh, lucho, asmadeus
Cc: linux_oss, v9fs, linux-kernel, security, stable, Yuhao Jiang
A buffer overflow vulnerability exists in the USB 9pfs transport layer
where inconsistent size validation between packet header parsing and
actual data copying allows a malicious USB host to overflow heap buffers.
The issue occurs because:
- usb9pfs_rx_header() validates only the declared size in packet header
- usb9pfs_rx_complete() uses req->actual (actual received bytes) for memcpy
This allows an attacker to craft packets with small declared size (bypassing
validation) but large actual payload (triggering overflow in memcpy).
Add validation in usb9pfs_rx_complete() to ensure req->actual does not
exceed the buffer capacity before copying data.
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Cc: stable@vger.kernel.org
Signed-off-by: Yuhao Jiang <danisjiang@gmail.com>
---
net/9p/trans_usbg.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index 6b694f117aef..047a2862fc84 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -242,6 +242,15 @@ static void usb9pfs_rx_complete(struct usb_ep *ep, struct usb_request *req)
if (!p9_rx_req)
return;
+ /* Validate actual received size against buffer capacity */
+ if (req->actual > p9_rx_req->rc.capacity) {
+ dev_err(&cdev->gadget->dev,
+ "received data size %u exceeds buffer capacity %zu\n",
+ req->actual, p9_rx_req->rc.capacity);
+ p9_req_put(usb9pfs->client, p9_rx_req);
+ return;
+ }
+
memcpy(p9_rx_req->rc.sdata, req->buf, req->actual);
p9_rx_req->rc.size = req->actual;
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net/9p: Fix buffer overflow in USB transport layer
2025-06-16 13:25 [PATCH] net/9p: Fix buffer overflow in USB transport layer Yuhao Jiang
@ 2025-06-16 23:00 ` asmadeus
2025-06-17 3:01 ` Danis Jiang
0 siblings, 1 reply; 6+ messages in thread
From: asmadeus @ 2025-06-16 23:00 UTC (permalink / raw)
To: Yuhao Jiang
Cc: ericvh, lucho, linux_oss, v9fs, linux-kernel, security, stable
Yuhao Jiang wrote on Mon, Jun 16, 2025 at 09:25:39PM +0800:
> A buffer overflow vulnerability exists in the USB 9pfs transport layer
> where inconsistent size validation between packet header parsing and
> actual data copying allows a malicious USB host to overflow heap buffers.
>
> The issue occurs because:
> - usb9pfs_rx_header() validates only the declared size in packet header
> - usb9pfs_rx_complete() uses req->actual (actual received bytes) for memcpy
>
> This allows an attacker to craft packets with small declared size (bypassing
> validation) but large actual payload (triggering overflow in memcpy).
>
> Add validation in usb9pfs_rx_complete() to ensure req->actual does not
> exceed the buffer capacity before copying data.
Thanks for this check!
Did you reproduce this or was this static analysis found?
(to knowi if you tested wrt question below)
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yuhao Jiang <danisjiang@gmail.com>
> ---
> net/9p/trans_usbg.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
> index 6b694f117aef..047a2862fc84 100644
> --- a/net/9p/trans_usbg.c
> +++ b/net/9p/trans_usbg.c
> @@ -242,6 +242,15 @@ static void usb9pfs_rx_complete(struct usb_ep *ep, struct usb_request *req)
> if (!p9_rx_req)
> return;
>
> + /* Validate actual received size against buffer capacity */
> + if (req->actual > p9_rx_req->rc.capacity) {
> + dev_err(&cdev->gadget->dev,
> + "received data size %u exceeds buffer capacity %zu\n",
> + req->actual, p9_rx_req->rc.capacity);
> + p9_req_put(usb9pfs->client, p9_rx_req);
I still haven't gotten around to setting up something to test this, and
even less the error case, but I'm not sure a single put is enough --
p9_client_cb does another put.
Conceptually I think it's better to mark the error and move on
e.g. (not even compile tested)
```
int status = REQ_STATUS_RCVD;
[...]
if (req->actual > p9_rx_req->rc.capacity) {
dev_err(...)
req->actual = 0;
status = REQ_STATUS_ERROR;
}
memcpy(..)
p9_rx_req->rc.size = req->actual;
p9_client_cb(usb9pfs->client, p9_rx_req, status);
p9_req_put(usb9pfs->client, p9_rx_req);
complete(&usb9pfs->received);
```
(I'm not sure overriding req->actual is allowed, might be safer to use
an intermediate variable like status instead)
What do you think?
Thanks,
--
Dominique Martinet | Asmadeus
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net/9p: Fix buffer overflow in USB transport layer
2025-06-16 23:00 ` asmadeus
@ 2025-06-17 3:01 ` Danis Jiang
2025-06-17 4:11 ` asmadeus
0 siblings, 1 reply; 6+ messages in thread
From: Danis Jiang @ 2025-06-17 3:01 UTC (permalink / raw)
To: asmadeus; +Cc: ericvh, lucho, linux_oss, v9fs, linux-kernel, security, stable
On Tue, Jun 17, 2025 at 7:00 AM <asmadeus@codewreck.org> wrote:
>
> Yuhao Jiang wrote on Mon, Jun 16, 2025 at 09:25:39PM +0800:
> > A buffer overflow vulnerability exists in the USB 9pfs transport layer
> > where inconsistent size validation between packet header parsing and
> > actual data copying allows a malicious USB host to overflow heap buffers.
> >
> > The issue occurs because:
> > - usb9pfs_rx_header() validates only the declared size in packet header
> > - usb9pfs_rx_complete() uses req->actual (actual received bytes) for memcpy
> >
> > This allows an attacker to craft packets with small declared size (bypassing
> > validation) but large actual payload (triggering overflow in memcpy).
> >
> > Add validation in usb9pfs_rx_complete() to ensure req->actual does not
> > exceed the buffer capacity before copying data.
>
> Thanks for this check!
>
> Did you reproduce this or was this static analysis found?
> (to knowi if you tested wrt question below)
I found this by static analysis.
>
> > Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> > Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Yuhao Jiang <danisjiang@gmail.com>
> > ---
> > net/9p/trans_usbg.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
> > index 6b694f117aef..047a2862fc84 100644
> > --- a/net/9p/trans_usbg.c
> > +++ b/net/9p/trans_usbg.c
> > @@ -242,6 +242,15 @@ static void usb9pfs_rx_complete(struct usb_ep *ep, struct usb_request *req)
> > if (!p9_rx_req)
> > return;
> >
> > + /* Validate actual received size against buffer capacity */
> > + if (req->actual > p9_rx_req->rc.capacity) {
> > + dev_err(&cdev->gadget->dev,
> > + "received data size %u exceeds buffer capacity %zu\n",
> > + req->actual, p9_rx_req->rc.capacity);
> > + p9_req_put(usb9pfs->client, p9_rx_req);
>
> I still haven't gotten around to setting up something to test this, and
> even less the error case, but I'm not sure a single put is enough --
> p9_client_cb does another put.
> Conceptually I think it's better to mark the error and move on
> e.g. (not even compile tested)
> ```
> int status = REQ_STATUS_RCVD;
>
> [...]
>
> if (req->actual > p9_rx_req->rc.capacity) {
> dev_err(...)
> req->actual = 0;
> status = REQ_STATUS_ERROR;
> }
>
> memcpy(..)
>
> p9_rx_req->rc.size = req->actual;
>
> p9_client_cb(usb9pfs->client, p9_rx_req, status);
> p9_req_put(usb9pfs->client, p9_rx_req);
>
> complete(&usb9pfs->received);
> ```
> (I'm not sure overriding req->actual is allowed, might be safer to use
> an intermediate variable like status instead)
>
> What do you think?
>
> Thanks,
> --
> Dominique Martinet | Asmadeus
Yes, I think your patch is better, my initial patch forgot p9_client_cb.
Thanks,
Yuhao Jiang
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net/9p: Fix buffer overflow in USB transport layer
2025-06-17 3:01 ` Danis Jiang
@ 2025-06-17 4:11 ` asmadeus
2025-06-17 4:29 ` Danis Jiang
0 siblings, 1 reply; 6+ messages in thread
From: asmadeus @ 2025-06-17 4:11 UTC (permalink / raw)
To: Danis Jiang
Cc: ericvh, lucho, linux_oss, v9fs, linux-kernel, security, stable,
Mirsad Todorovac
Danis Jiang wrote on Tue, Jun 17, 2025 at 11:01:40AM +0800:
>>> Add validation in usb9pfs_rx_complete() to ensure req->actual does not
>>> exceed the buffer capacity before copying data.
>>
>> Thanks for this check!
>>
>> Did you reproduce this or was this static analysis found?
>> (to knowi if you tested wrt question below)
>
> I found this by static analysis.
Ok.
>> I still haven't gotten around to setting up something to test this, and
>> even less the error case, but I'm not sure a single put is enough --
>> p9_client_cb does another put.
>> Conceptually I think it's better to mark the error and move on
>> e.g. (not even compile tested)
>> ```
>> int status = REQ_STATUS_RCVD;
>>
>> [...]
>>
>> if (req->actual > p9_rx_req->rc.capacity) {
>> dev_err(...)
>> req->actual = 0;
>> status = REQ_STATUS_ERROR;
>> }
>>
>> memcpy(..)
>>
>> p9_rx_req->rc.size = req->actual;
>>
>> p9_client_cb(usb9pfs->client, p9_rx_req, status);
>> p9_req_put(usb9pfs->client, p9_rx_req);
>>
>> complete(&usb9pfs->received);
>> ```
>> (I'm not sure overriding req->actual is allowed, might be safer to use
>> an intermediate variable like status instead)
>>
>> What do you think?
>
> Yes, I think your patch is better, my initial patch forgot p9_client_cb.
Ok, let's go with that then.
Would you like to resend "my" version, or should I do it (and
refer to your patch as Reported-by)?
Also if you resend let's add Mirsad Todorovac <mtodorovac69@gmail.com> too Ccs,
I've added him now.
(Mirsad, please check lore for full context if quote wasn't enough:
https://lkml.kernel.org/r/20250616132539.63434-1-danisjiang@gmail.com
)
Thanks,
--
Dominique Martinet | Asmadeus
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net/9p: Fix buffer overflow in USB transport layer
2025-06-17 4:11 ` asmadeus
@ 2025-06-17 4:29 ` Danis Jiang
0 siblings, 0 replies; 6+ messages in thread
From: Danis Jiang @ 2025-06-17 4:29 UTC (permalink / raw)
To: asmadeus
Cc: ericvh, lucho, linux_oss, v9fs, linux-kernel, security, stable,
Mirsad Todorovac
On Tue, Jun 17, 2025 at 12:12 PM <asmadeus@codewreck.org> wrote:
>
> Danis Jiang wrote on Tue, Jun 17, 2025 at 11:01:40AM +0800:
> >>> Add validation in usb9pfs_rx_complete() to ensure req->actual does not
> >>> exceed the buffer capacity before copying data.
> >>
> >> Thanks for this check!
> >>
> >> Did you reproduce this or was this static analysis found?
> >> (to knowi if you tested wrt question below)
> >
> > I found this by static analysis.
>
> Ok.
>
> >> I still haven't gotten around to setting up something to test this, and
> >> even less the error case, but I'm not sure a single put is enough --
> >> p9_client_cb does another put.
> >> Conceptually I think it's better to mark the error and move on
> >> e.g. (not even compile tested)
> >> ```
> >> int status = REQ_STATUS_RCVD;
> >>
> >> [...]
> >>
> >> if (req->actual > p9_rx_req->rc.capacity) {
> >> dev_err(...)
> >> req->actual = 0;
> >> status = REQ_STATUS_ERROR;
> >> }
> >>
> >> memcpy(..)
> >>
> >> p9_rx_req->rc.size = req->actual;
> >>
> >> p9_client_cb(usb9pfs->client, p9_rx_req, status);
> >> p9_req_put(usb9pfs->client, p9_rx_req);
> >>
> >> complete(&usb9pfs->received);
> >> ```
> >> (I'm not sure overriding req->actual is allowed, might be safer to use
> >> an intermediate variable like status instead)
> >>
> >> What do you think?
> >
> > Yes, I think your patch is better, my initial patch forgot p9_client_cb.
>
> Ok, let's go with that then.
>
> Would you like to resend "my" version, or should I do it (and
> refer to your patch as Reported-by)?
>
> Also if you resend let's add Mirsad Todorovac <mtodorovac69@gmail.com> too Ccs,
> I've added him now.
> (Mirsad, please check lore for full context if quote wasn't enough:
> https://lkml.kernel.org/r/20250616132539.63434-1-danisjiang@gmail.com
> )
>
>
> Thanks,
> --
> Dominique Martinet | Asmadeus
Sure, you can do it and add me as Reported-by, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] net/9p: Fix buffer overflow in USB transport layer
@ 2025-06-16 13:21 Yuhao Jiang
0 siblings, 0 replies; 6+ messages in thread
From: Yuhao Jiang @ 2025-06-16 13:21 UTC (permalink / raw)
To: danisjiang; +Cc: stable
A buffer overflow vulnerability exists in the USB 9pfs transport layer
where inconsistent size validation between packet header parsing and
actual data copying allows a malicious USB host to overflow heap buffers.
The issue occurs because:
- usb9pfs_rx_header() validates only the declared size in packet header
- usb9pfs_rx_complete() uses req->actual (actual received bytes) for memcpy
This allows an attacker to craft packets with small declared size (bypassing
validation) but large actual payload (triggering overflow in memcpy).
Add validation in usb9pfs_rx_complete() to ensure req->actual does not
exceed the buffer capacity before copying data.
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Cc: stable@vger.kernel.org
Signed-off-by: Yuhao Jiang <danisjiang@gmail.com>
---
net/9p/trans_usbg.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index 6b694f117aef..047a2862fc84 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -242,6 +242,15 @@ static void usb9pfs_rx_complete(struct usb_ep *ep, struct usb_request *req)
if (!p9_rx_req)
return;
+ /* Validate actual received size against buffer capacity */
+ if (req->actual > p9_rx_req->rc.capacity) {
+ dev_err(&cdev->gadget->dev,
+ "received data size %u exceeds buffer capacity %zu\n",
+ req->actual, p9_rx_req->rc.capacity);
+ p9_req_put(usb9pfs->client, p9_rx_req);
+ return;
+ }
+
memcpy(p9_rx_req->rc.sdata, req->buf, req->actual);
p9_rx_req->rc.size = req->actual;
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-06-17 4:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-16 13:25 [PATCH] net/9p: Fix buffer overflow in USB transport layer Yuhao Jiang
2025-06-16 23:00 ` asmadeus
2025-06-17 3:01 ` Danis Jiang
2025-06-17 4:11 ` asmadeus
2025-06-17 4:29 ` Danis Jiang
-- strict thread matches above, loose matches on Subject: below --
2025-06-16 13:21 Yuhao Jiang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox