[PATCH] USB: dummy-hcd: Fix uninitialized array use in init()

[PATCH] USB: dummy-hcd: Fix uninitialized array use in init()

[Bui Quang Minh]

This error path

	err_add_pdata:
		for (i = 0; i < mod_data.num; i++)
			kfree(dum[i]);

can be triggered when not all dum's elements are initialized.

Fix this by initializing all dum's elements to NULL.

Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 0eeaead..a2cf009 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -2734,7 +2734,7 @@ static int __init init(void)
 {
 	int	retval = -ENOMEM;
 	int	i;
-	struct	dummy *dum[MAX_NUM_UDC];
+	struct	dummy *dum[MAX_NUM_UDC] = {};
 
 	if (usb_disabled())
 		return -ENODEV;
-- 
2.7.4

Re: [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()

[Alan Stern]

On Fri, Dec 04, 2020 at 06:24:49AM +0000, Bui Quang Minh wrote:
> This error path
> 
> 	err_add_pdata:
> 		for (i = 0; i < mod_data.num; i++)
> 			kfree(dum[i]);
> 
> can be triggered when not all dum's elements are initialized.
> 
> Fix this by initializing all dum's elements to NULL.
> 
> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> ---
>  drivers/usb/gadget/udc/dummy_hcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index 0eeaead..a2cf009 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -2734,7 +2734,7 @@ static int __init init(void)
>  {
>  	int	retval = -ENOMEM;
>  	int	i;
> -	struct	dummy *dum[MAX_NUM_UDC];
> +	struct	dummy *dum[MAX_NUM_UDC] = {};
>  
>  	if (usb_disabled())
>  		return -ENODEV;

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

Does this initialization end up using less memory than an explicit 
memset() call?

Alan Stern

Re: [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()

[Minh Bùi Quang]

Vào Th 6, 4 thg 12, 2020 vào lúc 23:12 Alan Stern
<stern@rowland.harvard.edu> đã viết:
> Does this initialization end up using less memory than an explicit
> memset() call?

You mean speed? In my opinion, there is no difference in speed between 2 ways.
When I compile this array initialization using gcc 5.4.0,  this
initialization becomes
mov instructions when MAX_NUM_UDC=2 and becomes rep stos when
MAX_NUM_UDC=32. I think it makes no difference when comparing with memset()

Thanks,
Quang Minh

Re: [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()

[Alan Stern]

On Sat, Dec 05, 2020 at 07:47:01PM +0700, Minh Bùi Quang wrote:
> Vào Th 6, 4 thg 12, 2020 vào lúc 23:12 Alan Stern
> <stern@rowland.harvard.edu> đã viết:
> > Does this initialization end up using less memory than an explicit
> > memset() call?
> 
> You mean speed?

No, I mean memory space.

A memset call requires a certain amount of instruction space (to push 
the arguments and make the call) but no static data space.  
Initialization requires some instruction space (to copy the data) and 
static data space as well (to hold the data that is to be copied).

Alan Stern

> In my opinion, there is no difference in speed between 2 ways.
> When I compile this array initialization using gcc 5.4.0,  this
> initialization becomes
> mov instructions when MAX_NUM_UDC=2 and becomes rep stos when
> MAX_NUM_UDC=32. I think it makes no difference when comparing with memset()
> 
> Thanks,
> Quang Minh

Re: [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()

[Bui Quang Minh]

On Sat, Dec 05, 2020 at 10:15:11AM -0500, Alan Stern wrote:
> On Sat, Dec 05, 2020 at 07:47:01PM +0700, Minh Bùi Quang wrote:
> > Vào Th 6, 4 thg 12, 2020 vào lúc 23:12 Alan Stern
> > <stern@rowland.harvard.edu> đã viết:
> > > Does this initialization end up using less memory than an explicit
> > > memset() call?
> > 
> > You mean speed?
> 
> No, I mean memory space.
> 
> A memset call requires a certain amount of instruction space (to push 
> the arguments and make the call) but no static data space.  
> Initialization requires some instruction space (to copy the data) and 
> static data space as well (to hold the data that is to be copied).
> 
> Alan Stern
> 

Thank you for your clarification, I didn't think about it before.

As I check when compiling the code, with MAX_NUM_UDC=32 the initialization
becomes

        xor    eax,eax
        mov    ecx,0x40
        rep stos DWORD PTR es:[rdi],eax

With MAX_NUM_UDC=2, the initialization becomes

        mov    QWORD PTR [rbp-0x30],0x0
        mov    QWORD PTR [rbp-0x28],0x0

As I see, initialization does not require additional static data space.
Am I right?

Thanks,
Quang Minh 

Re: [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()

[Alan Stern]

On Sun, Dec 06, 2020 at 06:24:05PM +0700, Bui Quang Minh wrote:
> On Sat, Dec 05, 2020 at 10:15:11AM -0500, Alan Stern wrote:
> > On Sat, Dec 05, 2020 at 07:47:01PM +0700, Minh Bùi Quang wrote:
> > > Vào Th 6, 4 thg 12, 2020 vào lúc 23:12 Alan Stern
> > > <stern@rowland.harvard.edu> đã viết:
> > > > Does this initialization end up using less memory than an explicit
> > > > memset() call?
> > > 
> > > You mean speed?
> > 
> > No, I mean memory space.
> > 
> > A memset call requires a certain amount of instruction space (to push 
> > the arguments and make the call) but no static data space.  
> > Initialization requires some instruction space (to copy the data) and 
> > static data space as well (to hold the data that is to be copied).
> > 
> > Alan Stern
> > 
> 
> Thank you for your clarification, I didn't think about it before.
> 
> As I check when compiling the code, with MAX_NUM_UDC=32 the initialization
> becomes
> 
>         xor    eax,eax
>         mov    ecx,0x40
>         rep stos DWORD PTR es:[rdi],eax
> 
> With MAX_NUM_UDC=2, the initialization becomes
> 
>         mov    QWORD PTR [rbp-0x30],0x0
>         mov    QWORD PTR [rbp-0x28],0x0
> 
> As I see, initialization does not require additional static data space.
> Am I right?

Ah, okay, good.  I didn't realize the compiler was smart enough to do 
this.  Thanks for checking.

Alan Stern