[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2025062007-ravishing-overcrowd-7342@gregkh>
Date: Fri, 20 Jun 2025 06:56:24 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: asmadeus@...ewreck.org
Cc: Eric Van Hensbergen <ericvh@...nel.org>,
Latchesar Ionkov <lucho@...kov.net>,
Christian Schoenebeck <linux_oss@...debyte.com>,
Michael Grzeschik <m.grzeschik@...gutronix.de>,
stable@...r.kernel.org, Yuhao Jiang <danisjiang@...il.com>,
security@...nel.org, v9fs@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] net/9p: Fix buffer overflow in USB transport layer
On Fri, Jun 20, 2025 at 06:22:03AM +0900, Dominique Martinet via B4 Relay wrote:
> From: Dominique Martinet <asmadeus@...ewreck.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@...il.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@...r.kernel.org
> Signed-off-by: Dominique Martinet <asmadeus@...ewreck.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
Powered by blists - more mailing lists