diff mbox series

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

Message ID a8180973-3ded-3644-585a-169589a37642@omp.ru (mailing list archive)
State Accepted
Commit 4348f2e3ab3358898bfe93387ced7f3e1e3a6186
Headers show
Series usb: gadget: function: rndis: limit # of RNDIS instances to 1000 | expand

Commit Message

Sergey Shtylyov Aug. 23, 2022, 8:53 p.m. UTC
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(-)

Comments

Greg Kroah-Hartman Aug. 24, 2022, 5:54 a.m. UTC | #1
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
Sergey Shtylyov Aug. 25, 2022, 9:09 p.m. UTC | #2
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
diff mbox series

Patch

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)