Re: [PATCH] usb: gadget: configfs: NUL-terminate extended property data

[Neill Kapron]

> Unicode extended properties account for a UTF-16 terminator in
> ext_prop->data_len.  The configfs store path currently allocates only the
> input bytes, so ext_prop_data_show() copies one byte past the allocation.
> 
> Use kmemdup_nul() for the configfs data so the source byte implied by the
> terminator-inclusive length is present for both the configfs readback and
> the descriptor conversion.
> 
> Fixes: 7419485f197c ("usb: gadget: configfs: OS Extended Properties descriptors support")
> 
> Reported-by: Bingquan Chen <patzilla007@xxxxxxxxx>
> Link: https://lore.kernel.org/all/20260421141010.5607-1-patzilla007@xxxxxxxxx/
> Cc: stable@xxxxxxxxxxxxxxx
> 
> Tested: x86-64 KASAN configfs readback through a built-in RNDIS function
> instance.  The baseline reports a slab-out-of-bounds read in
> ext_prop_data_show(); the patched kernel reads back the expected 0x41 0x00
> bytes with no KASAN report.
> 
> Signed-off-by: Yingjie Wang <1075151112@xxxxxx>
> ---
>  drivers/usb/gadget/configfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
> index 51df6d1d1487..c302e8dd25a7 100644
> --- a/drivers/usb/gadget/configfs.c
> +++ b/drivers/usb/gadget/configfs.c
> @@ -1351,7 +1351,7 @@ static ssize_t ext_prop_data_store(struct config_item *item,
>  
>  	if (page[len - 1] == '\n' || page[len - 1] == '\0')
>  		--len;
> -	new_data = kmemdup(page, len, GFP_KERNEL);
> +	new_data = kmemdup_nul(page, len, GFP_KERNEL);

While this fixes one case, won't it cause ext_prop_data() show to report
a larger size with the trailing NUL?

The OOM still exists when switching types in ext_prop_type_store().

Can we add a couple of small helpers to convert between the raw payload
length and the wire length and use them in ext_prop_data_store/show,
and ext_prop_type_store to standardize and sanitize the length?

-- 
Neill Kapron <nkapron@xxxxxxxxxx>