Re: [PATCH v2 2/2 RESEND] usb: gadget: f_uac2: allow explicit channel count independent of channel mask

[kaf@nwlink.com]

On 2026-10-16 10:07, sashiko-bot@kernel.org wrote:
>> diff --git a/drivers/usb/gadget/function/f_uac2.c 
>> b/drivers/usb/gadget/function/f_uac2.c
>> index 46255ad0a9cfa..c7801bb8765e6 100644
>> --- a/drivers/usb/gadget/function/f_uac2.c
>> +++ b/drivers/usb/gadget/function/f_uac2.c
> [ ... ]
>> @@ -689,11 +699,11 @@ static int get_max_bw_for_bint(const struct 
>> f_uac2_opts *uac2_opts,
>>  		// Win10 requires max packet size + 1 frame
>>  		srate = srate * (1000 + uac2_opts->fb_max) / 1000;
>>  		// updated srate is always bigger, therefore DIV_ROUND_UP always 
>> yields +1
>> -		max_size_bw = num_channels(chmask) * ssize *
>> +		max_size_bw = channels * ssize *
>>  			(DIV_ROUND_UP(srate, factor / (1 << (bint - 1))));
>>  	} else {
>>  		// adding 1 frame provision for Win10
>> -		max_size_bw = num_channels(chmask) * ssize *
>> +		max_size_bw = channels * ssize *
>>  			(DIV_ROUND_UP(srate, factor / (1 << (bint - 1))) + 1);
>>  	}
>>  	return max_size_bw;
> 
> [Severity: High]
> Can this overflow the 16-bit capacity of max_size_bw?
> 
> With channel counts up to 255 independent of the channel mask, this 
> calculation
> can exceed 65535, causing the required bandwidth to be silently 
> truncated.

The max_size_bw truncation is a valid issue: expanded channel counts can
overflow the 16-bit calculation.  I plan to widen the bandwidth 
calculation
and comparison types in v3.  The existing warning-and-clamp behavior 
will be
preserved.



> [Severity: High]
> Does dynamically allocating memory and assigning it to the shared 
> static global
> out_feature_unit_desc overwrite prior instance allocations if multiple 
> UAC2
> instances are bound to a composite gadget via configfs, leaking memory?
> 
> [ ... ]
> [Severity: High]
> Does modifying static descriptor templates like usb_out_it_desc 
> directly
> during bind cause data races and corrupted descriptor fields if 
> concurrent
> bindings occur?

The two shared-global issues predate this series.  Regarding this patch
series, the changes do not alter descriptor ownership, lifetime, or
synchronization.  This series does not introduce or change any shared
descriptor state when constructing the descriptors.  The overall concern 
is
valid and separate from the explicit channel-count series presented 
here.


> [Severity: Critical]
> With the channel count expanded to 255, could this cause a heap buffer 
> overflow
> downstream in drivers/usb/gadget/function/u_audio.c?
> 
> When uac->p_framesize is dramatically increased by the larger channel 
> counts,
> and residue accumulation triggers in u_audio_iso_complete():
> 
> drivers/usb/gadget/function/u_audio.c:u_audio_iso_complete() {
>     ...
>     if ((unsigned int) div_result >= uac->p_framesize) {
>         req->length += uac->p_framesize;
>         uac->p_residue_mil -= uac->p_framesize * p_interval_mil;
>     ...
>     if (unlikely(pending < req->actual)) {
>         memcpy(req->buf, runtime->dma_area + hw_ptr, pending);
>         memcpy(req->buf + pending, runtime->dma_area, req->actual - 
> pending);
>     ...
> }
> 
> Will this memcpy overflow req->buf by up to p_framesize because 
> req->length
> was increased without verifying it stays within ep->maxpacket?
> 
> Request buffers are tightly packed at intervals of ep->maxpacket:
> 
> drivers/usb/gadget/function/u_audio.c:u_audio_start_playback() {
>     ...
>     req->buf = prm->rbuf + i * ep->maxpacket;
>     ...
> }
> 
> This means any overflow immediately corrupts the next request's active 
> DMA
> memory.

The overflow in u_audio_iso_complete(): this is very interesting.  I'm 
able
to achieve an oversized request using existing channel mask 
configuration
(27 channels, 4 bytes/sample, 19200 Hz sampling rate, p_hs_bint=3).  The
endpoint bandwidth calculation reported a required packet size of 1080
bytes, exceeding the 1024-byte high-speed endpoint limit.

I instrumented the driver to detect oversized requests and clamp the 
request
length before the memcpy. The warning confirmed that req->length 
exceeded
ep->maxpacket by 56 bytes, although the test requires a bandwidth-
insufficient endpoint configuration.

This demonstrates that the underlying bounds issue is reachable without 
the
new channel-count attributes. Looks like this will expand to a 4-patch 
series.
The u_audio_iso_complete() patch may need to stand on its own ahead of 
the other
patches.

Thanks,
Kyle