[PATCH] usb: usbtest: reject transfer lengths the allocator cannot serve

[PATCH] usb: usbtest: reject transfer lengths the allocator cannot serve

[YuanZhe Liu]

usbtest sizes every buffer it allocates from the user supplied
struct usbtest_param::length, but it never bounds that value.  The
scatterlist tests, the queued bulk tests and the unlink tests pass the
length straight to kzalloc() and usb_alloc_coherent(), so a large value
reaches the page allocator as an allocation order above MAX_PAGE_ORDER:

    WARNING: mm/page_alloc.c:5340 at alloc_order_allowed mm/page_alloc.c:5340 [inline]
    WARNING: mm/page_alloc.c:5340 at __alloc_frozen_pages_noprof+0x4ae/0x1fd0 mm/page_alloc.c:5397
    ...
     alloc_sglist+0x331/0x500
     usbtest_do_ioctl+0x2686/0x5880
     usbtest_ioctl+0x440/0x720

A run of "TEST 7:  write/-7 -2147483648 sglists 3 entries
0..-2147483645 bytes" asks kzalloc() for more than 2 GiB.  The queued
bulk tests reach the same warning through usb_alloc_coherent() and
dma_direct_alloc().  The allocation fails after the warning and the
driver then returns -ENOMEM, but the warning itself is driven by
unvalidated user input.

Reject such lengths where the other parameter limits are already
enforced.  KMALLOC_MAX_SIZE, that is PAGE_SIZE << MAX_PAGE_ORDER, is
the largest contiguous allocation the page allocator is willing to
serve, and the unaligned tests add at most one byte on top of the
requested length.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Co-developed-by: Bin Yu <byu@xidian.edu.cn>
Signed-off-by: Bin Yu <byu@xidian.edu.cn>
Co-developed-by: Cheng Wen <wencheng@xidian.edu.cn>
Signed-off-by: Cheng Wen <wencheng@xidian.edu.cn>
Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>
---
 drivers/usb/misc/usbtest.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 8759df49be28..8f2a4752b9c8 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -2192,6 +2192,15 @@ usbtest_do_ioctl(struct usb_interface *intf, struct usbtest_param_32 *param)
 		return -EINVAL;
 	if (param->sglen > MAX_SGLEN)
 		return -EINVAL;
+	/*
+	 * Every test case sizes its transfer buffers from param->length,
+	 * using one contiguous allocation per URB or scatterlist entry.
+	 * The page allocator cannot serve a length of KMALLOC_MAX_SIZE
+	 * or more: the allocation order would exceed MAX_PAGE_ORDER and
+	 * the allocator would warn before failing the allocation.
+	 */
+	if (param->length >= KMALLOC_MAX_SIZE)
+		return -EINVAL;
 	/*
 	 * Just a bunch of test cases that every HCD is expected to handle.
 	 *
-- 
2.34.1

Re: [PATCH] usb: usbtest: reject transfer lengths the allocator cannot serve

[sashiko-bot]

> usbtest sizes every buffer it allocates from the user supplied
> struct usbtest_param::length, but it never bounds that value.  The
> scatterlist tests, the queued bulk tests and the unlink tests pass the
> length straight to kzalloc() and usb_alloc_coherent(), so a large value
> reaches the page allocator as an allocation order above MAX_PAGE_ORDER:
> 
>     WARNING: mm/page_alloc.c:5340 at alloc_order_allowed mm/page_alloc.c:5340 [inline]
>     WARNING: mm/page_alloc.c:5340 at __alloc_frozen_pages_noprof+0x4ae/0x1fd0 mm/page_alloc.c:5397
>     ...
>      alloc_sglist+0x331/0x500
>      usbtest_do_ioctl+0x2686/0x5880
>      usbtest_ioctl+0x440/0x720
> 
> A run of "TEST 7:  write/-7 -2147483648 sglists 3 entries
> 0..-2147483645 bytes" asks kzalloc() for more than 2 GiB.  The queued
> [ ... ]
> Co-developed-by: Bin Yu <byu@xidian.edu.cn>
> Signed-off-by: Bin Yu <byu@xidian.edu.cn>
> Co-developed-by: Cheng Wen <wencheng@xidian.edu.cn>
> Signed-off-by: Cheng Wen <wencheng@xidian.edu.cn>
> Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016094848.38533-1-25031212351@stu.xidian.edu.cn?part=1

Re: [PATCH] usb: usbtest: reject transfer lengths the allocator cannot serve

[Alan Stern]

On Fri, Oct 16, 2026 at 05:48:48PM +0800, YuanZhe Liu wrote:
> usbtest sizes every buffer it allocates from the user supplied
> struct usbtest_param::length, but it never bounds that value.  The
> scatterlist tests, the queued bulk tests and the unlink tests pass the
> length straight to kzalloc() and usb_alloc_coherent(), so a large value
> reaches the page allocator as an allocation order above MAX_PAGE_ORDER:
> 
>     WARNING: mm/page_alloc.c:5340 at alloc_order_allowed mm/page_alloc.c:5340 [inline]
>     WARNING: mm/page_alloc.c:5340 at __alloc_frozen_pages_noprof+0x4ae/0x1fd0 mm/page_alloc.c:5397
>     ...
>      alloc_sglist+0x331/0x500
>      usbtest_do_ioctl+0x2686/0x5880
>      usbtest_ioctl+0x440/0x720
> 
> A run of "TEST 7:  write/-7 -2147483648 sglists 3 entries
> 0..-2147483645 bytes" asks kzalloc() for more than 2 GiB.  The queued
> bulk tests reach the same warning through usb_alloc_coherent() and
> dma_direct_alloc().  The allocation fails after the warning and the
> driver then returns -ENOMEM, but the warning itself is driven by
> unvalidated user input.
> 
> Reject such lengths where the other parameter limits are already
> enforced.  KMALLOC_MAX_SIZE, that is PAGE_SIZE << MAX_PAGE_ORDER, is
> the largest contiguous allocation the page allocator is willing to
> serve, and the unaligned tests add at most one byte on top of the
> requested length.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Co-developed-by: Bin Yu <byu@xidian.edu.cn>
> Signed-off-by: Bin Yu <byu@xidian.edu.cn>
> Co-developed-by: Cheng Wen <wencheng@xidian.edu.cn>
> Signed-off-by: Cheng Wen <wencheng@xidian.edu.cn>
> Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>
> ---
>  drivers/usb/misc/usbtest.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index 8759df49be28..8f2a4752b9c8 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -2192,6 +2192,15 @@ usbtest_do_ioctl(struct usb_interface *intf, struct usbtest_param_32 *param)
>  		return -EINVAL;
>  	if (param->sglen > MAX_SGLEN)
>  		return -EINVAL;
> +	/*
> +	 * Every test case sizes its transfer buffers from param->length,
> +	 * using one contiguous allocation per URB or scatterlist entry.
> +	 * The page allocator cannot serve a length of KMALLOC_MAX_SIZE
> +	 * or more: the allocation order would exceed MAX_PAGE_ORDER and
> +	 * the allocator would warn before failing the allocation.
> +	 */

This comment is longer than it needs to be.  You can just say:

	/* Don't allow unreasonably large transfer-buffer allocations */

The details about contiguous allocation per URB, MAX_PAGE_ORDER, and 
allocator warnings don't really matter here.

Aside from that,

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern

> +	if (param->length >= KMALLOC_MAX_SIZE)
> +		return -EINVAL;
>  	/*
>  	 * Just a bunch of test cases that every HCD is expected to handle.
>  	 *
> -- 
> 2.34.1

[PATCH v2] usb: usbtest: reject transfer lengths the allocator cannot serve

[YuanZhe Liu]

usbtest sizes every buffer it allocates from the user supplied
struct usbtest_param::length, but it never bounds that value.  The
scatterlist tests, the queued bulk tests and the unlink tests pass the
length straight to kzalloc() and usb_alloc_coherent(), so a large value
reaches the page allocator as an allocation order above MAX_PAGE_ORDER:

    WARNING: mm/page_alloc.c:5340 at alloc_order_allowed mm/page_alloc.c:5340 [inline]
    WARNING: mm/page_alloc.c:5340 at __alloc_frozen_pages_noprof+0x4ae/0x1fd0 mm/page_alloc.c:5397
    ...
     alloc_sglist+0x331/0x500
     usbtest_do_ioctl+0x2686/0x5880
     usbtest_ioctl+0x440/0x720

A run of "TEST 7:  write/-7 -2147483648 sglists 3 entries
0..-2147483645 bytes" asks kzalloc() for more than 2 GiB.  The queued
bulk tests reach the same warning through usb_alloc_coherent() and
dma_direct_alloc().  The allocation fails after the warning and the
driver then returns -ENOMEM, but the warning itself is driven by
unvalidated user input.

Reject such lengths where the other parameter limits are already
enforced.  KMALLOC_MAX_SIZE, that is PAGE_SIZE << MAX_PAGE_ORDER, is
the largest contiguous allocation the page allocator is willing to
serve, and the unaligned tests add at most one byte on top of the
requested length.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Co-developed-by: Bin Yu <byu@xidian.edu.cn>
Signed-off-by: Bin Yu <byu@xidian.edu.cn>
Co-developed-by: Cheng Wen <wencheng@xidian.edu.cn>
Signed-off-by: Cheng Wen <wencheng@xidian.edu.cn>
Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>

---
v2:
 - Shortened the comment above the length check, as suggested by Alan Stern.
 - Added the Acked-by from Alan Stern.
---
 drivers/usb/misc/usbtest.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 8759df49be28..6a3394800fd8 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -2192,6 +2192,9 @@ usbtest_do_ioctl(struct usb_interface *intf, struct usbtest_param_32 *param)
 		return -EINVAL;
 	if (param->sglen > MAX_SGLEN)
 		return -EINVAL;
+	/* Don't allow unreasonably large transfer-buffer allocations */
+	if (param->length >= KMALLOC_MAX_SIZE)
+		return -EINVAL;
 	/*
 	 * Just a bunch of test cases that every HCD is expected to handle.
 	 *
-- 
2.34.1

Re: [PATCH v2] usb: usbtest: reject transfer lengths the allocator cannot serve

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] usb: division by zero in simple_io() when length is 0
- [Medium] usb: usbtest: incomplete limit in iso_alloc_urb allows MAX_PAGE_ORDER bypass
--

commit 805535b4eccf7c27a2100576fca5ef6fb016a6bd
Author: YuanZhe Liu <25031212351@stu.xidian.edu.cn>

usb: usbtest: reject transfer lengths the allocator cannot serve

This commit restricts the length parameter of usbtest ioctls to ensure it
doesn't exceed KMALLOC_MAX_SIZE. It aims to fix warnings from the page
allocator when a large parameter length triggers allocations above
MAX_PAGE_ORDER.

> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index 8759df49be287..6a3394800fd86 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -2192,6 +2192,9 @@ usbtest_do_ioctl(struct usb_interface *intf, struct usbtest_param_32 *param)
>  		return -EINVAL;
>  	if (param->sglen > MAX_SGLEN)
>  		return -EINVAL;
> +	/* Don't allow unreasonably large transfer-buffer allocations */
> +	if (param->length >= KMALLOC_MAX_SIZE)
> +		return -EINVAL;

[Severity: High]
Does this check also need to ensure param->length > 0?

If userspace provides param->length == 0 and param->vary != 0 for test cases
3 or 4, simple_alloc_urb() succeeds and returns an URB with
transfer_buffer_length = 0.

When simple_io() is called:

drivers/usb/misc/usbtest.c:simple_io() {
    ...
	int max = urb->transfer_buffer_length; /* max becomes 0 */
    ...
		if (vary) {
			int len = urb->transfer_buffer_length;

			len += vary;
			len %= max;
    ...
}

Can this division by zero trigger a kernel panic?

[Severity: Medium]
Is this upper bound check sufficient for isochronous transfers in tests 15
and 16?

For these tests, test_queue() calls iso_alloc_urb() which allocates the URB
based on the number of packets:

drivers/usb/misc/usbtest.c:iso_alloc_urb() {
    ...
	maxp = usb_endpoint_maxp(desc);
    ...
	packets = DIV_ROUND_UP(bytes, maxp);
	urb = usb_alloc_urb(packets, GFP_KERNEL);
    ...
}

If a malicious or dummy USB device provides an isochronous endpoint with a
small maxp (e.g., 1), packets will equal param->length. usb_alloc_urb() will
then allocate memory sized for the URB and its packet descriptors:
sizeof(struct urb) + packets * sizeof(struct usb_iso_packet_descriptor).
This is roughly 192 + param->length * 16 bytes.

If param->length is just below KMALLOC_MAX_SIZE, the resulting allocation
size passed to kmalloc() will be roughly 16 times KMALLOC_MAX_SIZE. Does
this exceed MAX_PAGE_ORDER and trigger the exact same warning this patch
attempts to fix?

>  	/*
>  	 * Just a bunch of test cases that every HCD is expected to handle.
>  	 *

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016154707.13098-1-25031212351@stu.xidian.edu.cn?part=1