* [PATCH -mm] drivers/usb/core/config.c: kzalloc(0,..)
@ 2007-05-08 3:03 Dan Kruchinin
2007-05-08 11:32 ` Jiri Slaby
2007-05-08 14:14 ` Greg KH
0 siblings, 2 replies; 6+ messages in thread
From: Dan Kruchinin @ 2007-05-08 3:03 UTC (permalink / raw)
To: linux-kernel, Andrew Morton
The following patch fixes such SLUB report(when someone tries to
allocate 0 bytes):
--
May 8 00:19:15 midgard kernel: [ 21.933467] BUG: at
include/linux/slub_def.h:88 kmalloc_index()
May 8 00:19:15 midgard kernel: [ 21.933470]
[show_registers+410/736] show_trace_log_lvl+0x1a/0x30
May 8 00:19:15 midgard kernel: [ 21.933478]
[print_trace_warning_symbol+50/64] show_trace+0x12/0x20
May 8 00:19:15 midgard kernel: [ 21.933482] [fixup_irqs+38/192]
dump_stack+0x16/0x20
May 8 00:19:15 midgard kernel: [ 21.933485] [do_lookup+195/400]
get_slab+0x213/0x230
May 8 00:19:15 midgard kernel: [ 21.933489] [do_lookup+309/400]
__kmalloc_track_caller+0x15/0x40
May 8 00:19:15 midgard kernel: [ 21.933493] [__vunmap+25/240]
__kzalloc+0x19/0x50
May 8 00:19:15 midgard kernel: [ 21.933498] [<df05778e>]
usb_parse_configuration+0x85e/0xe70 [usbcore]
May 8 00:19:15 midgard kernel: [ 21.933520] [<df057fcb>]
usb_get_configuration+0x12b/0x450 [usbcore]
May 8 00:19:15 midgard kernel: [ 21.933535] [<df04ed87>]
usb_new_device+0x17/0x1c0 [usbcore]
May 8 00:19:15 midgard kernel: [ 21.933550] [<df05073a>]
hub_thread+0x79a/0xfd0 [usbcore]
May 8 00:19:15 midgard kernel: [ 21.933564]
[run_posix_cpu_timers+1218/2064] kthread+0x42/0x70
May 8 00:19:15 midgard kernel: [ 21.933569] [math_error+87/240]
kernel_thread_helper+0x7/0x10
May 8 00:19:15 midgard kernel: [ 21.933572] =======================
--
The problem was in drivers/usb/core/config.c in function usb_parse_interface:
---
num_ep = num_ep_orig = alt->desc.bNumEndpoints;
...
len = sizeof(struct usb_host_endpoint) * num_ep;
alt->endpoint = kzalloc(len, GFP_KERNEL);
---
num_ep can be 0, as it was in my case, so following patch makes this
situation more obvious
and clear.
--------------
diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index bfb3731..4db6b21 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -185,7 +185,10 @@ static int usb_parse_interface(struct device
*ddev, int cfgno,
num_ep = USB_MAXENDPOINTS;
}
- len = sizeof(struct usb_host_endpoint) * num_ep;
+ len = sizeof(struct usb_host_endpoint);
+ if (num_ep > 0)
+ len *= num_ep;
+
alt->endpoint = kzalloc(len, GFP_KERNEL);
if (!alt->endpoint)
return -ENOMEM;
---------------
Dan Kruchinin.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -mm] drivers/usb/core/config.c: kzalloc(0,..)
2007-05-08 3:03 [PATCH -mm] drivers/usb/core/config.c: kzalloc(0,..) Dan Kruchinin
@ 2007-05-08 11:32 ` Jiri Slaby
2007-05-08 14:14 ` Greg KH
1 sibling, 0 replies; 6+ messages in thread
From: Jiri Slaby @ 2007-05-08 11:32 UTC (permalink / raw)
To: Dan Kruchinin; +Cc: linux-kernel, Andrew Morton, linux-usb-devel, Greg KH
Dan Kruchinin napsal(a):
> The following patch fixes such SLUB report(when someone tries to
> allocate 0 bytes):
> --
> May 8 00:19:15 midgard kernel: [ 21.933467] BUG: at
> include/linux/slub_def.h:88 kmalloc_index()
> May 8 00:19:15 midgard kernel: [ 21.933470]
> [show_registers+410/736] show_trace_log_lvl+0x1a/0x30
> May 8 00:19:15 midgard kernel: [ 21.933478]
> [print_trace_warning_symbol+50/64] show_trace+0x12/0x20
> May 8 00:19:15 midgard kernel: [ 21.933482] [fixup_irqs+38/192]
> dump_stack+0x16/0x20
> May 8 00:19:15 midgard kernel: [ 21.933485] [do_lookup+195/400]
> get_slab+0x213/0x230
> May 8 00:19:15 midgard kernel: [ 21.933489] [do_lookup+309/400]
> __kmalloc_track_caller+0x15/0x40
> May 8 00:19:15 midgard kernel: [ 21.933493] [__vunmap+25/240]
> __kzalloc+0x19/0x50
> May 8 00:19:15 midgard kernel: [ 21.933498] [<df05778e>]
> usb_parse_configuration+0x85e/0xe70 [usbcore]
> May 8 00:19:15 midgard kernel: [ 21.933520] [<df057fcb>]
> usb_get_configuration+0x12b/0x450 [usbcore]
> May 8 00:19:15 midgard kernel: [ 21.933535] [<df04ed87>]
> usb_new_device+0x17/0x1c0 [usbcore]
> May 8 00:19:15 midgard kernel: [ 21.933550] [<df05073a>]
> hub_thread+0x79a/0xfd0 [usbcore]
> May 8 00:19:15 midgard kernel: [ 21.933564]
> [run_posix_cpu_timers+1218/2064] kthread+0x42/0x70
> May 8 00:19:15 midgard kernel: [ 21.933569] [math_error+87/240]
> kernel_thread_helper+0x7/0x10
> May 8 00:19:15 midgard kernel: [ 21.933572] =======================
> --
>
> The problem was in drivers/usb/core/config.c in function
> usb_parse_interface:
> ---
> num_ep = num_ep_orig = alt->desc.bNumEndpoints;
> ...
> len = sizeof(struct usb_host_endpoint) * num_ep;
> alt->endpoint = kzalloc(len, GFP_KERNEL);
> ---
>
> num_ep can be 0, as it was in my case, so following patch makes this
> situation more obvious
> and clear.
>
> --------------
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index bfb3731..4db6b21 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -185,7 +185,10 @@ static int usb_parse_interface(struct device
> *ddev, int cfgno,
> num_ep = USB_MAXENDPOINTS;
> }
>
> - len = sizeof(struct usb_host_endpoint) * num_ep;
> + len = sizeof(struct usb_host_endpoint);
> + if (num_ep > 0)
> + len *= num_ep;
> +
> alt->endpoint = kzalloc(len, GFP_KERNEL);
> if (!alt->endpoint)
> return -ENOMEM;
> ---------------
I don't think, this is correct, but let USB people make a decision (always CC
subsys maintainers plus people who might be involved).
regards,
--
http://www.fi.muni.cz/~xslaby/ Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8 22A0 32CC 55C3 39D4 7A7E
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -mm] drivers/usb/core/config.c: kzalloc(0,..)
2007-05-08 3:03 [PATCH -mm] drivers/usb/core/config.c: kzalloc(0,..) Dan Kruchinin
2007-05-08 11:32 ` Jiri Slaby
@ 2007-05-08 14:14 ` Greg KH
2007-05-08 15:57 ` [linux-usb-devel] [PATCH -mm] drivers/usb/core/config.c: kzalloc(0, ..) Alan Stern
1 sibling, 1 reply; 6+ messages in thread
From: Greg KH @ 2007-05-08 14:14 UTC (permalink / raw)
To: Dan Kruchinin; +Cc: linux-kernel, Andrew Morton, linux-usb-devel
On Tue, May 08, 2007 at 07:03:08AM +0400, Dan Kruchinin wrote:
> The following patch fixes such SLUB report(when someone tries to
> allocate 0 bytes):
> --
> May 8 00:19:15 midgard kernel: [ 21.933467] BUG: at
> include/linux/slub_def.h:88 kmalloc_index()
> May 8 00:19:15 midgard kernel: [ 21.933470]
> [show_registers+410/736] show_trace_log_lvl+0x1a/0x30
> May 8 00:19:15 midgard kernel: [ 21.933478]
> [print_trace_warning_symbol+50/64] show_trace+0x12/0x20
> May 8 00:19:15 midgard kernel: [ 21.933482] [fixup_irqs+38/192]
> dump_stack+0x16/0x20
> May 8 00:19:15 midgard kernel: [ 21.933485] [do_lookup+195/400]
> get_slab+0x213/0x230
> May 8 00:19:15 midgard kernel: [ 21.933489] [do_lookup+309/400]
> __kmalloc_track_caller+0x15/0x40
> May 8 00:19:15 midgard kernel: [ 21.933493] [__vunmap+25/240]
> __kzalloc+0x19/0x50
> May 8 00:19:15 midgard kernel: [ 21.933498] [<df05778e>]
> usb_parse_configuration+0x85e/0xe70 [usbcore]
> May 8 00:19:15 midgard kernel: [ 21.933520] [<df057fcb>]
> usb_get_configuration+0x12b/0x450 [usbcore]
> May 8 00:19:15 midgard kernel: [ 21.933535] [<df04ed87>]
> usb_new_device+0x17/0x1c0 [usbcore]
> May 8 00:19:15 midgard kernel: [ 21.933550] [<df05073a>]
> hub_thread+0x79a/0xfd0 [usbcore]
> May 8 00:19:15 midgard kernel: [ 21.933564]
> [run_posix_cpu_timers+1218/2064] kthread+0x42/0x70
> May 8 00:19:15 midgard kernel: [ 21.933569] [math_error+87/240]
> kernel_thread_helper+0x7/0x10
> May 8 00:19:15 midgard kernel: [ 21.933572] =======================
> --
>
> The problem was in drivers/usb/core/config.c in function
> usb_parse_interface:
> ---
> num_ep = num_ep_orig = alt->desc.bNumEndpoints;
> ...
> len = sizeof(struct usb_host_endpoint) * num_ep;
> alt->endpoint = kzalloc(len, GFP_KERNEL);
> ---
>
> num_ep can be 0, as it was in my case, so following patch makes this
> situation more obvious
> and clear.
Well, the whitespace was damaged, but the idea looks sane. Care to
respin this?
However, what kind of device do you have that you have no endpoints for
an interface? Why have the interface then?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [linux-usb-devel] [PATCH -mm] drivers/usb/core/config.c: kzalloc(0, ..)
2007-05-08 14:14 ` Greg KH
@ 2007-05-08 15:57 ` Alan Stern
2007-05-08 16:45 ` Randy Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2007-05-08 15:57 UTC (permalink / raw)
To: Dan Kruchinin
Cc: Greg KH, Andrew Morton, Kernel development list,
USB development list
On Tue, 8 May 2007, Greg KH wrote:
> > The problem was in drivers/usb/core/config.c in function
> > usb_parse_interface:
> > ---
> > num_ep = num_ep_orig = alt->desc.bNumEndpoints;
> > ...
> > len = sizeof(struct usb_host_endpoint) * num_ep;
> > alt->endpoint = kzalloc(len, GFP_KERNEL);
> > ---
> >
> > num_ep can be 0, as it was in my case, so following patch makes this
> > situation more obvious
> > and clear.
How about instead just doing:
+ num_ep = max(num_ep, 1);
len = sizeof(struct usb_host_endpoint) * num_ep;
Also, wasn't it true at one point that it was legal to call kmalloc() with
a length of 0? ISTR seeing somewhere that it's true for regular malloc().
Alan Stern
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [linux-usb-devel] [PATCH -mm] drivers/usb/core/config.c: kzalloc(0, ..)
2007-05-08 15:57 ` [linux-usb-devel] [PATCH -mm] drivers/usb/core/config.c: kzalloc(0, ..) Alan Stern
@ 2007-05-08 16:45 ` Randy Dunlap
2007-05-10 6:41 ` Dan Kruchinin
0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2007-05-08 16:45 UTC (permalink / raw)
To: Alan Stern
Cc: Dan Kruchinin, Greg KH, Andrew Morton, Kernel development list,
USB development list
On Tue, 8 May 2007 11:57:07 -0400 (EDT) Alan Stern wrote:
> On Tue, 8 May 2007, Greg KH wrote:
>
> > > The problem was in drivers/usb/core/config.c in function
> > > usb_parse_interface:
> > > ---
> > > num_ep = num_ep_orig = alt->desc.bNumEndpoints;
> > > ...
> > > len = sizeof(struct usb_host_endpoint) * num_ep;
> > > alt->endpoint = kzalloc(len, GFP_KERNEL);
> > > ---
> > >
> > > num_ep can be 0, as it was in my case, so following patch makes this
> > > situation more obvious
> > > and clear.
>
> How about instead just doing:
>
> + num_ep = max(num_ep, 1);
> len = sizeof(struct usb_host_endpoint) * num_ep;
>
> Also, wasn't it true at one point that it was legal to call kmalloc() with
> a length of 0? ISTR seeing somewhere that it's true for regular malloc().
kmalloc(0) was legal with CONFIG_SLAB=y. However, there is now
something called SLUB, which just returned an error when size == 0.
It has recently been modified to mirror the SLAB behavior but also
do a stack dump so that "bad" callers can be fixed.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [linux-usb-devel] [PATCH -mm] drivers/usb/core/config.c: kzalloc(0, ..)
2007-05-08 16:45 ` Randy Dunlap
@ 2007-05-10 6:41 ` Dan Kruchinin
0 siblings, 0 replies; 6+ messages in thread
From: Dan Kruchinin @ 2007-05-10 6:41 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel, linux-usb-devel
On 5/8/07, Randy Dunlap <rdunlap@xenotime.net> wrote:
> On Tue, 8 May 2007 11:57:07 -0400 (EDT) Alan Stern wrote:
>
> > On Tue, 8 May 2007, Greg KH wrote:
> >
> > > > The problem was in drivers/usb/core/config.c in function
> > > > usb_parse_interface:
> > > > ---
> > > > num_ep = num_ep_orig = alt->desc.bNumEndpoints;
> > > > ...
> > > > len = sizeof(struct usb_host_endpoint) * num_ep;
> > > > alt->endpoint = kzalloc(len, GFP_KERNEL);
> > > > ---
> > > >
> > > > num_ep can be 0, as it was in my case, so following patch makes this
> > > > situation more obvious
> > > > and clear.
> >
> > How about instead just doing:
> >
> > + num_ep = max(num_ep, 1);
> > len = sizeof(struct usb_host_endpoint) * num_ep;
> >
> > Also, wasn't it true at one point that it was legal to call kmalloc() with
> > a length of 0? ISTR seeing somewhere that it's true for regular malloc().
>
> kmalloc(0) was legal with CONFIG_SLAB=y. However, there is now
> something called SLUB, which just returned an error when size == 0.
SLUB works correctly with kmalloc(0) too, but it calls
WARN_ON_ONCE(size == 0);
in include/linux/slub_def.h: kmalloc_index.
btw: as I know when kmalloc(0) both slub and slab allocate the
smallest possible size. Can this size be smaller than sizeof(struct
usb_host_endpoint)? If it is, may it be a problem?
thanks.
Dan Kruchinin.
> It has recently been modified to mirror the SLAB behavior but also
> do a stack dump so that "bad" callers can be fixed.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-05-10 6:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-08 3:03 [PATCH -mm] drivers/usb/core/config.c: kzalloc(0,..) Dan Kruchinin
2007-05-08 11:32 ` Jiri Slaby
2007-05-08 14:14 ` Greg KH
2007-05-08 15:57 ` [linux-usb-devel] [PATCH -mm] drivers/usb/core/config.c: kzalloc(0, ..) Alan Stern
2007-05-08 16:45 ` Randy Dunlap
2007-05-10 6:41 ` Dan Kruchinin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox