[PATCH] usb: gadget: function: rndis: limit # of RNDIS instances to 1000

[PATCH] usb: gadget: function: rndis: limit # of RNDIS instances to 1000

[Sergey Shtylyov]

As follows from #define NAME_TEMPLATE, the procfs code in the RNDIS driver
expects the # of instances to be 3-digit decimal, while the driver calls
ida_simple_get() passing 0 as the 'end' argument which results in actual
max instance # of INT_MAX.  Limit the maximum # of RNDIS instances to 1000
which is still a lot! :-)

Found by Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

---
This patch is against the 'next' branch of Felipe Balbi's 'usb.git' repo...

 drivers/usb/gadget/function/rndis.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: usb/drivers/usb/gadget/function/rndis.c
===================================================================
--- usb.orig/drivers/usb/gadget/function/rndis.c
+++ usb/drivers/usb/gadget/function/rndis.c
@@ -865,7 +865,7 @@ EXPORT_SYMBOL_GPL(rndis_msg_parser);
 
 static inline int rndis_get_nr(void)
 {
-	return ida_simple_get(&rndis_ida, 0, 0, GFP_KERNEL);
+	return ida_simple_get(&rndis_ida, 0, 1000, GFP_KERNEL);
 }
 
 static inline void rndis_put_nr(int nr)

Re: [PATCH] usb: gadget: function: rndis: limit # of RNDIS instances to 1000

[Greg Kroah-Hartman]

On Tue, Aug 23, 2022 at 11:53:26PM +0300, Sergey Shtylyov wrote:
> As follows from #define NAME_TEMPLATE, the procfs code in the RNDIS driver
> expects the # of instances to be 3-digit decimal, while the driver calls
> ida_simple_get() passing 0 as the 'end' argument which results in actual
> max instance # of INT_MAX.  Limit the maximum # of RNDIS instances to 1000
> which is still a lot! :-)
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> ---
> This patch is against the 'next' branch of Felipe Balbi's 'usb.git' repo...
> 
>  drivers/usb/gadget/function/rndis.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: usb/drivers/usb/gadget/function/rndis.c
> ===================================================================
> --- usb.orig/drivers/usb/gadget/function/rndis.c
> +++ usb/drivers/usb/gadget/function/rndis.c
> @@ -865,7 +865,7 @@ EXPORT_SYMBOL_GPL(rndis_msg_parser);
>  
>  static inline int rndis_get_nr(void)
>  {
> -	return ida_simple_get(&rndis_ida, 0, 0, GFP_KERNEL);
> +	return ida_simple_get(&rndis_ida, 0, 1000, GFP_KERNEL);

Why not just change the procfs code instead?  It's not like anyone
should ever be using this driver anyway.  We should delete it soon, it's
totally broken and insecure as noted in the past :(

thanks,

greg k-h

Re: [PATCH] usb: gadget: function: rndis: limit # of RNDIS instances to 1000

[Sergey Shtylyov]

Hello!

On 8/24/22 8:54 AM, Greg Kroah-Hartman wrote:

>> As follows from #define NAME_TEMPLATE, the procfs code in the RNDIS driver
>> expects the # of instances to be 3-digit decimal, while the driver calls
>> ida_simple_get() passing 0 as the 'end' argument which results in actual
>> max instance # of INT_MAX.  Limit the maximum # of RNDIS instances to 1000
>> which is still a lot! :-)
>>
>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>> analysis tool.
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>
>> ---
>> This patch is against the 'next' branch of Felipe Balbi's 'usb.git' repo...
>>
>>  drivers/usb/gadget/function/rndis.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Index: usb/drivers/usb/gadget/function/rndis.c
>> ===================================================================
>> --- usb.orig/drivers/usb/gadget/function/rndis.c
>> +++ usb/drivers/usb/gadget/function/rndis.c
>> @@ -865,7 +865,7 @@ EXPORT_SYMBOL_GPL(rndis_msg_parser);
>>  
>>  static inline int rndis_get_nr(void)
>>  {
>> -	return ida_simple_get(&rndis_ida, 0, 0, GFP_KERNEL);
>> +	return ida_simple_get(&rndis_ida, 0, 1000, GFP_KERNEL);
> 
> Why not just change the procfs code instead?

   You mean changing #define NAME_TEMPLATE from "driver/rndis-%03d" to
"driver/rndis-%010d" and then changing the size of the name[] buffers to
24 bytes?

> It's not like anyone should ever be using this driver anyway.
> We should delete it soon, it's
> totally broken and insecure as noted in the past :(

   Oh, I wasn't aware of that... I just got the SVACE reports tossed
at me by the ISP people...

> thanks,
> 
> greg k-h

MBR, Sergey