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

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

[Dominique Martinet via B4 Relay]

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);
+		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;
+
+	p9_client_cb(usb9pfs->client, p9_rx_req, status);
 	p9_req_put(usb9pfs->client, p9_rx_req);
 
 	complete(&usb9pfs->received);

---
base-commit: 74b4cc9b8780bfe8a3992c9ac0033bf22ac01f19
change-id: 20250620-9p-usb_overflow-25bfc5e9bef3

Best regards,
-- 
Dominique Martinet <asmadeus@codewreck.org>

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

[Greg Kroah-Hartman]

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

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

[kernel test robot]

Hi Dominique,

kernel test robot noticed the following build errors:

[auto build test ERROR on 74b4cc9b8780bfe8a3992c9ac0033bf22ac01f19]

url:    https://github.com/intel-lab-lkp/linux/commits/Dominique-Martinet-via-B4-Relay/net-9p-Fix-buffer-overflow-in-USB-transport-layer/20250620-052411
base:   74b4cc9b8780bfe8a3992c9ac0033bf22ac01f19
patch link:    https://lore.kernel.org/r/20250620-9p-usb_overflow-v2-1-026c6109c7a1%40codewreck.org
patch subject: [PATCH v2] net/9p: Fix buffer overflow in USB transport layer
config: i386-randconfig-004-20250620 (https://download.01.org/0day-ci/archive/20250620/202506201706.IUsC9LOI-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250620/202506201706.IUsC9LOI-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506201706.IUsC9LOI-lkp@intel.com/

All errors (new ones prefixed by >>):

>> net/9p/trans_usbg.c:257:23: error: use of undeclared identifier 'req_sizel'; did you mean 'req_size'?
     257 |         p9_rx_req->rc.size = req_sizel;
         |                              ^~~~~~~~~
         |                              req_size
   net/9p/trans_usbg.c:234:15: note: 'req_size' declared here
     234 |         unsigned int req_size = req->actual;
         |                      ^
   1 error generated.


vim +257 net/9p/trans_usbg.c

   228	
   229	static void usb9pfs_rx_complete(struct usb_ep *ep, struct usb_request *req)
   230	{
   231		struct f_usb9pfs *usb9pfs = ep->driver_data;
   232		struct usb_composite_dev *cdev = usb9pfs->function.config->cdev;
   233		struct p9_req_t *p9_rx_req;
   234		unsigned int req_size = req->actual;
   235		int status = REQ_STATUS_RCVD;
   236	
   237		if (req->status) {
   238			dev_err(&cdev->gadget->dev, "%s usb9pfs complete --> %d, %d/%d\n",
   239				ep->name, req->status, req->actual, req->length);
   240			return;
   241		}
   242	
   243		p9_rx_req = usb9pfs_rx_header(usb9pfs, req->buf);
   244		if (!p9_rx_req)
   245			return;
   246	
   247		if (req_size > p9_rx_req->rc.capacity) {
   248			dev_err(&cdev->gadget->dev,
   249				"%s received data size %u exceeds buffer capacity %zu\n",
   250				ep->name, req_size, p9_rx_req->rc.capacity);
   251			req_size = 0;
   252			status = REQ_STATUS_ERROR;
   253		}
   254	
   255		memcpy(p9_rx_req->rc.sdata, req->buf, req_size);
   256	
 > 257		p9_rx_req->rc.size = req_sizel;
   258	
   259		p9_client_cb(usb9pfs->client, p9_rx_req, status);
   260		p9_req_put(usb9pfs->client, p9_rx_req);
   261	
   262		complete(&usb9pfs->received);
   263	}
   264	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Dominique Martinet]

Greg Kroah-Hartman wrote on Fri, Jun 20, 2025 at 06:56:24AM +0200:
> > -	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.

I realize I hadn't replied to this one -- I (still) consider 9p mounts
to be somewhat privileged/trusted, so I'm fine flooding kernel logs with
a broken device.
If the trust model changes (I've been askedto make 9p mountable by
non-root users... perhaps after we've caught up with syzcallers
reports but not holding my breath) then we can revisit this, but 9p IO
errors are rather badly behaved afaik (connection possibly never
recovers) so I'd rather the first error stands out.

> > -	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/sorry for this one as well :/

-- 
Dominique