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,
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
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 :/
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-Rel... base: 74b4cc9b8780bfe8a3992c9ac0033bf22ac01f19 patch link: https://lore.kernel.org/r/20250620-9p-usb_overflow-v2-1-026c6109c7a1%40codew... 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@i...) 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@i...)
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
linux-stable-mirror@lists.linaro.org