| Message ID | 1460106483-24793-1-git-send-email-chunfeng.yun@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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 ? >> 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 ?
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.
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. odd, I have never seen this problem running testusb with the same arguments.
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
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
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; }
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@mediatek.com> --- drivers/usb/core/buffer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)