linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()
@ 2020-12-04  6:24 Bui Quang Minh
  2020-12-04 16:12 ` Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: Bui Quang Minh @ 2020-12-04  6:24 UTC (permalink / raw)
  Cc: Bui Quang Minh, Felipe Balbi, Greg Kroah-Hartman, Corentin Labbe,
	Alan Stern, Jules Irenge, Andrey Konovalov, linux-usb,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()
  2020-12-04  6:24 [PATCH] USB: dummy-hcd: Fix uninitialized array use in init() Bui Quang Minh
@ 2020-12-04 16:12 ` Alan Stern
  2020-12-05 12:47   ` Minh Bùi Quang
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2020-12-04 16:12 UTC (permalink / raw)
  To: Bui Quang Minh
  Cc: Felipe Balbi, Greg Kroah-Hartman, Corentin Labbe, Jules Irenge,
	Andrey Konovalov, linux-usb, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()
  2020-12-04 16:12 ` Alan Stern
@ 2020-12-05 12:47   ` Minh Bùi Quang
  2020-12-05 15:15     ` Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: Minh Bùi Quang @ 2020-12-05 12:47 UTC (permalink / raw)
  To: Alan Stern
  Cc: Felipe Balbi, Greg Kroah-Hartman, Corentin Labbe, Jules Irenge,
	Andrey Konovalov, linux-usb, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()
  2020-12-05 12:47   ` Minh Bùi Quang
@ 2020-12-05 15:15     ` Alan Stern
  2020-12-06 11:24       ` Bui Quang Minh
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2020-12-05 15:15 UTC (permalink / raw)
  To: Minh Bùi Quang
  Cc: Felipe Balbi, Greg Kroah-Hartman, Corentin Labbe, Jules Irenge,
	Andrey Konovalov, linux-usb, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()
  2020-12-05 15:15     ` Alan Stern
@ 2020-12-06 11:24       ` Bui Quang Minh
  2020-12-06 16:07         ` Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: Bui Quang Minh @ 2020-12-06 11:24 UTC (permalink / raw)
  To: Alan Stern
  Cc: Felipe Balbi, Greg Kroah-Hartman, Corentin Labbe, Jules Irenge,
	Andrey Konovalov, linux-usb, linux-kernel

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 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] USB: dummy-hcd: Fix uninitialized array use in init()
  2020-12-06 11:24       ` Bui Quang Minh
@ 2020-12-06 16:07         ` Alan Stern
  0 siblings, 0 replies; 6+ messages in thread
From: Alan Stern @ 2020-12-06 16:07 UTC (permalink / raw)
  To: Bui Quang Minh
  Cc: Felipe Balbi, Greg Kroah-Hartman, Corentin Labbe, Jules Irenge,
	Andrey Konovalov, linux-usb, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-12-06 16:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-04  6:24 [PATCH] USB: dummy-hcd: Fix uninitialized array use in init() Bui Quang Minh
2020-12-04 16:12 ` Alan Stern
2020-12-05 12:47   ` Minh Bùi Quang
2020-12-05 15:15     ` Alan Stern
2020-12-06 11:24       ` Bui Quang Minh
2020-12-06 16:07         ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).