[PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[Chunfeng Yun]

NULL pointer dereferrence will happen when class driver
wants to allocate zero length buffer and pool_max[0]
can't be used, so skip reserved pool in this case.

Signed-off-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/usb/core/buffer.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 2741566..c8f958b 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -131,7 +131,7 @@ void *hcd_buffer_alloc(
 	}
 
 	for (i = 0; i < HCD_BUFFER_POOLS; i++) {
-		if (size <= pool_max[i])
+		if (pool_max[i] && size <= pool_max[i])
 			return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
 	}
 	return dma_alloc_coherent(hcd->self.controller, size, dma, mem_flags);
@@ -158,7 +158,7 @@ void hcd_buffer_free(
 	}
 
 	for (i = 0; i < HCD_BUFFER_POOLS; i++) {
-		if (size <= pool_max[i]) {
+		if (pool_max[i] && size <= pool_max[i]) {
 			dma_pool_free(hcd->pool[i], addr, dma);
 			return;
 		}
-- 
1.7.9.5

Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[Greg Kroah-Hartman]

On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote:
> NULL pointer dereferrence will happen when class driver
> wants to allocate zero length buffer and pool_max[0]
> can't be used, so skip reserved pool in this case.

Why would a driver want to allocate a 0 length buffer?  What driver does
this?

Shouldn't we fix that issue instead?

thanks,

greg k-h

Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[Alan Stern]

On Fri, 8 Apr 2016, Greg Kroah-Hartman wrote:

> On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote:
> > NULL pointer dereferrence will happen when class driver
> > wants to allocate zero length buffer and pool_max[0]
> > can't be used, so skip reserved pool in this case.
> 
> Why would a driver want to allocate a 0 length buffer?  What driver does
> this?
> 
> Shouldn't we fix that issue instead?

And even if a driver does want to allocate a 0-length buffer, shouldn't 
the function simply return early instead of running through all its 
calculations?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[Greg Kroah-Hartman]

On Fri, Apr 08, 2016 at 11:21:05AM -0400, Alan Stern wrote:
> On Fri, 8 Apr 2016, Greg Kroah-Hartman wrote:
> 
> > On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote:
> > > NULL pointer dereferrence will happen when class driver
> > > wants to allocate zero length buffer and pool_max[0]
> > > can't be used, so skip reserved pool in this case.
> > 
> > Why would a driver want to allocate a 0 length buffer?  What driver does
> > this?
> > 
> > Shouldn't we fix that issue instead?
> 
> And even if a driver does want to allocate a 0-length buffer, shouldn't 
> the function simply return early instead of running through all its 
> calculations?

Yes it should, Chunfeng, can you fix this patch to do that please.

thanks,

greg k-h

Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[chunfeng yun]

Hi,

On Tue, 2016-04-26 at 16:11 -0700, Greg Kroah-Hartman wrote:
> On Fri, Apr 08, 2016 at 11:21:05AM -0400, Alan Stern wrote:
> > On Fri, 8 Apr 2016, Greg Kroah-Hartman wrote:
> > 
> > > On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote:
> > > > NULL pointer dereferrence will happen when class driver
> > > > wants to allocate zero length buffer and pool_max[0]
> > > > can't be used, so skip reserved pool in this case.
> > > 
> > > Why would a driver want to allocate a 0 length buffer?  What driver does
> > > this?
> > > 
> > > Shouldn't we fix that issue instead?
> > 
> > And even if a driver does want to allocate a 0-length buffer, shouldn't 
> > the function simply return early instead of running through all its 
> > calculations?
> 
> Yes it should, Chunfeng, can you fix this patch to do that please.
> 
Ok, I will fix it later

> thanks,
> 
> greg k-h


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[chunfeng yun]

On Fri, 2016-04-08 at 07:07 -0700, Greg Kroah-Hartman wrote:
> On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote:
> > NULL pointer dereferrence will happen when class driver
> > wants to allocate zero length buffer and pool_max[0]
> > can't be used, so skip reserved pool in this case.
> 
> Why would a driver want to allocate a 0 length buffer?  What driver does
> this?
It's misc/usbtest.c
> 
> Shouldn't we fix that issue instead?
I don't know which way is better, but it seems simple to fix it up in
buffer.c 
> 
> thanks,
> 
> greg k-h

Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 912 bytes --]


Hi,

chunfeng yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> writes:
> On Fri, 2016-04-08 at 07:07 -0700, Greg Kroah-Hartman wrote:
>> On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote:
>> > NULL pointer dereferrence will happen when class driver
>> > wants to allocate zero length buffer and pool_max[0]
>> > can't be used, so skip reserved pool in this case.
>> 
>> Why would a driver want to allocate a 0 length buffer?  What driver does
>> this?
> It's misc/usbtest.c

that'll do what you ask it to do with the userspace tool testusb. Are
you trying to pass a size of 0 ?

>> Shouldn't we fix that issue instead?
> I don't know which way is better, but it seems simple to fix it up in
> buffer.c 

I think we should, really, avoid a 0-length allocation, but passing a
size of 0 to testusb isn't very good either ;-) How are you calling
testusb ?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[chunfeng yun]

On Mon, 2016-04-11 at 08:07 +0300, Felipe Balbi wrote:
> Hi,
> 
> chunfeng yun <chunfeng.yun@mediatek.com> writes:
> > On Fri, 2016-04-08 at 07:07 -0700, Greg Kroah-Hartman wrote:
> >> On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote:
> >> > NULL pointer dereferrence will happen when class driver
> >> > wants to allocate zero length buffer and pool_max[0]
> >> > can't be used, so skip reserved pool in this case.
> >> 
> >> Why would a driver want to allocate a 0 length buffer?  What driver does
> >> this?
> > It's misc/usbtest.c
> 
> that'll do what you ask it to do with the userspace tool testusb. Are
> you trying to pass a size of 0 ?
> 
No, I just ran "testusb -t10" which called test_ctrl_queue().
In this function, sub-case 8 passed a parameter @len as 0 to
simple_alloc_urb(), and then it tried to allocate a 0-length buffer.

> >> Shouldn't we fix that issue instead?
> > I don't know which way is better, but it seems simple to fix it up in
> > buffer.c 
> 
> I think we should, really, avoid a 0-length allocation, but passing a
> size of 0 to testusb isn't very good either ;-) How are you calling
> testusb ?
> 
As explained above.

Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]


Hi,

chunfeng yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> writes:
>> > On Fri, 2016-04-08 at 07:07 -0700, Greg Kroah-Hartman wrote:
>> >> On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote:
>> >> > NULL pointer dereferrence will happen when class driver
>> >> > wants to allocate zero length buffer and pool_max[0]
>> >> > can't be used, so skip reserved pool in this case.
>> >> 
>> >> Why would a driver want to allocate a 0 length buffer?  What driver does
>> >> this?
>> > It's misc/usbtest.c
>> 
>> that'll do what you ask it to do with the userspace tool testusb. Are
>> you trying to pass a size of 0 ?
>> 
> No, I just ran "testusb -t10" which called test_ctrl_queue().
> In this function, sub-case 8 passed a parameter @len as 0 to
> simple_alloc_urb(), and then it tried to allocate a 0-length buffer.

odd, I have never seen this problem running testusb with the same
arguments.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence

[Alan Stern]

On Mon, 11 Apr 2016, chunfeng yun wrote:

> On Mon, 2016-04-11 at 08:07 +0300, Felipe Balbi wrote:
> > Hi,
> > 
> > chunfeng yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> writes:
> > > On Fri, 2016-04-08 at 07:07 -0700, Greg Kroah-Hartman wrote:
> > >> On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote:
> > >> > NULL pointer dereferrence will happen when class driver
> > >> > wants to allocate zero length buffer and pool_max[0]
> > >> > can't be used, so skip reserved pool in this case.
> > >> 
> > >> Why would a driver want to allocate a 0 length buffer?  What driver does
> > >> this?
> > > It's misc/usbtest.c
> > 
> > that'll do what you ask it to do with the userspace tool testusb. Are
> > you trying to pass a size of 0 ?
> > 
> No, I just ran "testusb -t10" which called test_ctrl_queue().
> In this function, sub-case 8 passed a parameter @len as 0 to
> simple_alloc_urb(), and then it tried to allocate a 0-length buffer.

That should be easy enough to fix.  Just skip the calls that allocate 
the buffer if the length would be 0.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html