Re: [PATCH] net/9p: Fix buffer overflow in USB transport layer

From: Danis Jiang
Date: Tue Jun 17 2025 - 00:30:23 EST


On Tue, Jun 17, 2025 at 12:12 PM <asmadeus@xxxxxxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx
> )
>
>
> Thanks,
> --
> Dominique Martinet | Asmadeus

Sure, you can do it and add me as Reported-by, thanks!