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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Fri, Jun 20, 2025 at 06:22:03AM +0900, Dominique Martinet via B4 Relay wrote:
> From: Dominique Martinet <asmadeus@codewreck.org>
> 
> 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>
> Closes: https://lkml.kernel.org/r/20250616132539.63434-1-danisjiang@gmail.com
> Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
> ---
> Not actually tested, I'll try to find time to figure out how to run with
> qemu for real this time...
> 
> Changes in v2:
> - run through p9_client_cb() on error
> - Link to v1: https://lore.kernel.org/r/20250616132539.63434-1-danisjiang@gmail.com
> ---
>  net/9p/trans_usbg.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
> index 6b694f117aef296a66419fed5252305e7a1d0936..43078e0d4ca3f4063660f659d28452c81bef10b4 100644
> --- a/net/9p/trans_usbg.c
> +++ b/net/9p/trans_usbg.c
> @@ -231,6 +231,8 @@ 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_req_t *p9_rx_req;
> +	unsigned int req_size = req->actual;
> +	int status = REQ_STATUS_RCVD;
>  
>  	if (req->status) {
>  		dev_err(&cdev->gadget->dev, "%s usb9pfs complete --> %d, %d/%d\n",
> @@ -242,11 +244,19 @@ static void usb9pfs_rx_complete(struct usb_ep *ep, struct usb_request *req)
>  	if (!p9_rx_req)
>  		return;
>  
> -	memcpy(p9_rx_req->rc.sdata, req->buf, req->actual);
> +	if (req_size > p9_rx_req->rc.capacity) {
> +		dev_err(&cdev->gadget->dev,
> +			"%s received data size %u exceeds buffer capacity %zu\n",
> +			ep->name, req_size, p9_rx_req->rc.capacity);

Do you want a broken device to be able to flood the kernel log?  You
might want to change this to dev_dbg() instead.



> +		req_size = 0;
> +		status = REQ_STATUS_ERROR;
> +	}
>  
> -	p9_rx_req->rc.size = req->actual;
> +	memcpy(p9_rx_req->rc.sdata, req->buf, req_size);
>  
> -	p9_client_cb(usb9pfs->client, p9_rx_req, REQ_STATUS_RCVD);
> +	p9_rx_req->rc.size = req_sizel;

Did this code build properly?

thanks,

greg k-h