2022-06-29 08:36:40

by Sebin Sebastian

[permalink] [raw]
Subject: [PATCH -next] usb: gadget: dereference before null check

Fix coverity warning dereferencing before null check. _ep and desc is
dereferenced on all paths until the check for null. Move the
initializations after the check for null.
Coverity issue: 1518209

Signed-off-by: SebinSebastian <[email protected]>
---
drivers/usb/gadget/udc/aspeed_udc.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c
index d75a4e070bf7..96f8193fca15 100644
--- a/drivers/usb/gadget/udc/aspeed_udc.c
+++ b/drivers/usb/gadget/udc/aspeed_udc.c
@@ -341,10 +341,6 @@ static void ast_udc_stop_activity(struct ast_udc_dev *udc)
static int ast_udc_ep_enable(struct usb_ep *_ep,
const struct usb_endpoint_descriptor *desc)
{
- u16 maxpacket = usb_endpoint_maxp(desc);
- struct ast_udc_ep *ep = to_ast_ep(_ep);
- struct ast_udc_dev *udc = ep->udc;
- u8 epnum = usb_endpoint_num(desc);
unsigned long flags;
u32 ep_conf = 0;
u8 dir_in;
@@ -356,6 +352,11 @@ static int ast_udc_ep_enable(struct usb_ep *_ep,
return -EINVAL;
}

+ u16 maxpacket = usb_endpoint_maxp(desc);
+ struct ast_udc_ep *ep = to_ast_ep(_ep);
+ struct ast_udc_dev *udc = ep->udc;
+ u8 epnum = usb_endpoint_num(desc);
+
if (!udc->driver) {
EP_DBG(ep, "bogus device state\n");
return -ESHUTDOWN;
--
2.34.1


2022-06-29 09:07:31

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH -next] usb: gadget: dereference before null check

On Wed, Jun 29, 2022 at 01:37:25PM +0530, SebinSebastian wrote:
> Fix coverity warning dereferencing before null check. _ep and desc is
> dereferenced on all paths until the check for null. Move the
> initializations after the check for null.
> Coverity issue: 1518209
>
> Signed-off-by: SebinSebastian <[email protected]>
> ---
> drivers/usb/gadget/udc/aspeed_udc.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c
> index d75a4e070bf7..96f8193fca15 100644
> --- a/drivers/usb/gadget/udc/aspeed_udc.c
> +++ b/drivers/usb/gadget/udc/aspeed_udc.c
> @@ -341,10 +341,6 @@ static void ast_udc_stop_activity(struct ast_udc_dev *udc)
> static int ast_udc_ep_enable(struct usb_ep *_ep,
> const struct usb_endpoint_descriptor *desc)
> {
> - u16 maxpacket = usb_endpoint_maxp(desc);
> - struct ast_udc_ep *ep = to_ast_ep(_ep);
> - struct ast_udc_dev *udc = ep->udc;
> - u8 epnum = usb_endpoint_num(desc);
> unsigned long flags;
> u32 ep_conf = 0;
> u8 dir_in;
> @@ -356,6 +352,11 @@ static int ast_udc_ep_enable(struct usb_ep *_ep,
> return -EINVAL;
> }
>
> + u16 maxpacket = usb_endpoint_maxp(desc);
> + struct ast_udc_ep *ep = to_ast_ep(_ep);
> + struct ast_udc_dev *udc = ep->udc;
> + u8 epnum = usb_endpoint_num(desc);
> +
> if (!udc->driver) {
> EP_DBG(ep, "bogus device state\n");
> return -ESHUTDOWN;
> --
> 2.34.1
>

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch breaks the build.

- Your patch contains warnings and/or errors noticed by the
scripts/checkpatch.pl tool.

- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for what needs to be done
here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

2022-06-29 12:17:15

by Sebin Sebastian

[permalink] [raw]
Subject: Re: [PATCH -next] usb: gadget: dereference before null check

On Wed, Jun 29, 2022 at 10:24:07AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jun 29, 2022 at 01:37:25PM +0530, SebinSebastian wrote:
> > Fix coverity warning dereferencing before null check. _ep and desc is
> > dereferenced on all paths until the check for null. Move the
> > initializations after the check for null.
> > Coverity issue: 1518209
> >
> > Signed-off-by: SebinSebastian <[email protected]>
> > ---
> > drivers/usb/gadget/udc/aspeed_udc.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c
> > index d75a4e070bf7..96f8193fca15 100644
> > --- a/drivers/usb/gadget/udc/aspeed_udc.c
> > +++ b/drivers/usb/gadget/udc/aspeed_udc.c
> > @@ -341,10 +341,6 @@ static void ast_udc_stop_activity(struct ast_udc_dev *udc)
> > static int ast_udc_ep_enable(struct usb_ep *_ep,
> > const struct usb_endpoint_descriptor *desc)
> > {
> > - u16 maxpacket = usb_endpoint_maxp(desc);
> > - struct ast_udc_ep *ep = to_ast_ep(_ep);
> > - struct ast_udc_dev *udc = ep->udc;
> > - u8 epnum = usb_endpoint_num(desc);
> > unsigned long flags;
> > u32 ep_conf = 0;
> > u8 dir_in;
> > @@ -356,6 +352,11 @@ static int ast_udc_ep_enable(struct usb_ep *_ep,
> > return -EINVAL;
> > }
> >
> > + u16 maxpacket = usb_endpoint_maxp(desc);
> > + struct ast_udc_ep *ep = to_ast_ep(_ep);
> > + struct ast_udc_dev *udc = ep->udc;
> > + u8 epnum = usb_endpoint_num(desc);
> > +
> > if (!udc->driver) {
> > EP_DBG(ep, "bogus device state\n");
> > return -ESHUTDOWN;
> > --
> > 2.34.1
> >
>
> Hi,
>
> This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
> a patch that has triggered this response. He used to manually respond
> to these common problems, but in order to save his sanity (he kept
> writing the same thing over and over, yet to different people), I was
> created. Hopefully you will not take offence and will fix the problem
> in your patch and resubmit it so that it can be accepted into the Linux
> kernel tree.
>
> You are receiving this message because of the following common error(s)
> as indicated below:
>
> - Your patch breaks the build.
>
> - Your patch contains warnings and/or errors noticed by the
> scripts/checkpatch.pl tool.
>
> - This looks like a new version of a previously submitted patch, but you
> did not list below the --- line any changes from the previous version.
> Please read the section entitled "The canonical patch format" in the
> kernel file, Documentation/SubmittingPatches for what needs to be done
> here to properly describe this.
>
> If you wish to discuss this problem further, or you have questions about
> how to resolve this issue, please feel free to respond to this email and
> Greg will reply once he has dug out from the pending patches received
> from other developers.
>
> thanks,
>
> greg k-h's patch email bot

I am sorry to keep on bothering with this incorrect patches. I am
running the checkpatch script everytime before I sent any patches. It is
not showing any warnings or errors. Is it because of my name that my
patches are getting rejected? I can see a space missing.

2022-06-29 12:54:55

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH -next] usb: gadget: dereference before null check

Hi SebinSebastian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20220628]

url: https://github.com/intel-lab-lkp/linux/commits/SebinSebastian/usb-gadget-dereference-before-null-check/20220629-161008
base: cb71b93c2dc36d18a8b05245973328d018272cdf
config: mips-allyesconfig
compiler: mips-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/97ebbd93f269a58b3b5a003898d6e09c29a73ab0
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review SebinSebastian/usb-gadget-dereference-before-null-check/20220629-161008
git checkout 97ebbd93f269a58b3b5a003898d6e09c29a73ab0
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/usb/gadget/udc/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/usb/gadget/udc/aspeed_udc.c: In function 'ast_udc_ep_enable':
drivers/usb/gadget/udc/aspeed_udc.c:349:22: error: 'ep' undeclared (first use in this function); did you mean '_ep'?
349 | if (!_ep || !ep || !desc || desc->bDescriptorType != USB_DT_ENDPOINT ||
| ^~
| _ep
drivers/usb/gadget/udc/aspeed_udc.c:349:22: note: each undeclared identifier is reported only once for each function it appears in
drivers/usb/gadget/udc/aspeed_udc.c:350:13: error: 'maxpacket' undeclared (first use in this function)
350 | maxpacket == 0 || maxpacket > ep->ep.maxpacket) {
| ^~~~~~~~~
>> drivers/usb/gadget/udc/aspeed_udc.c:355:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
355 | u16 maxpacket = usb_endpoint_maxp(desc);
| ^~~


vim +355 drivers/usb/gadget/udc/aspeed_udc.c

340
341 static int ast_udc_ep_enable(struct usb_ep *_ep,
342 const struct usb_endpoint_descriptor *desc)
343 {
344 unsigned long flags;
345 u32 ep_conf = 0;
346 u8 dir_in;
347 u8 type;
348
349 if (!_ep || !ep || !desc || desc->bDescriptorType != USB_DT_ENDPOINT ||
350 maxpacket == 0 || maxpacket > ep->ep.maxpacket) {
351 EP_DBG(ep, "Failed, invalid EP enable param\n");
352 return -EINVAL;
353 }
354
> 355 u16 maxpacket = usb_endpoint_maxp(desc);
356 struct ast_udc_ep *ep = to_ast_ep(_ep);
357 struct ast_udc_dev *udc = ep->udc;
358 u8 epnum = usb_endpoint_num(desc);
359
360 if (!udc->driver) {
361 EP_DBG(ep, "bogus device state\n");
362 return -ESHUTDOWN;
363 }
364
365 EP_DBG(ep, "maxpacket:0x%x\n", maxpacket);
366
367 spin_lock_irqsave(&udc->lock, flags);
368
369 ep->desc = desc;
370 ep->stopped = 0;
371 ep->ep.maxpacket = maxpacket;
372 ep->chunk_max = AST_EP_DMA_DESC_MAX_LEN;
373
374 if (maxpacket < AST_UDC_EPn_MAX_PACKET)
375 ep_conf = EP_SET_MAX_PKT(maxpacket);
376
377 ep_conf |= EP_SET_EP_NUM(epnum);
378
379 type = usb_endpoint_type(desc);
380 dir_in = usb_endpoint_dir_in(desc);
381 ep->dir_in = dir_in;
382 if (!ep->dir_in)
383 ep_conf |= EP_DIR_OUT;
384
385 EP_DBG(ep, "type %d, dir_in %d\n", type, dir_in);
386 switch (type) {
387 case USB_ENDPOINT_XFER_ISOC:
388 ep_conf |= EP_SET_TYPE_MASK(EP_TYPE_ISO);
389 break;
390
391 case USB_ENDPOINT_XFER_BULK:
392 ep_conf |= EP_SET_TYPE_MASK(EP_TYPE_BULK);
393 break;
394
395 case USB_ENDPOINT_XFER_INT:
396 ep_conf |= EP_SET_TYPE_MASK(EP_TYPE_INT);
397 break;
398 }
399
400 ep->desc_mode = udc->desc_mode && ep->descs_dma && ep->dir_in;
401 if (ep->desc_mode) {
402 ast_ep_write(ep, EP_DMA_CTRL_RESET, AST_UDC_EP_DMA_CTRL);
403 ast_ep_write(ep, 0, AST_UDC_EP_DMA_STS);
404 ast_ep_write(ep, ep->descs_dma, AST_UDC_EP_DMA_BUFF);
405
406 /* Enable Long Descriptor Mode */
407 ast_ep_write(ep, EP_DMA_CTRL_IN_LONG_MODE | EP_DMA_DESC_MODE,
408 AST_UDC_EP_DMA_CTRL);
409
410 ep->descs_wptr = 0;
411
412 } else {
413 ast_ep_write(ep, EP_DMA_CTRL_RESET, AST_UDC_EP_DMA_CTRL);
414 ast_ep_write(ep, EP_DMA_SINGLE_STAGE, AST_UDC_EP_DMA_CTRL);
415 ast_ep_write(ep, 0, AST_UDC_EP_DMA_STS);
416 }
417
418 /* Cleanup data toggle just in case */
419 ast_udc_write(udc, EP_TOGGLE_SET_EPNUM(epnum), AST_VHUB_EP_DATA);
420
421 /* Enable EP */
422 ast_ep_write(ep, ep_conf | EP_ENABLE, AST_UDC_EP_CONFIG);
423
424 EP_DBG(ep, "ep_config: 0x%x\n", ast_ep_read(ep, AST_UDC_EP_CONFIG));
425
426 spin_unlock_irqrestore(&udc->lock, flags);
427
428 return 0;
429 }
430

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (5.20 kB)
config (328.57 kB)
Download all attachments

2022-06-29 16:06:52

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH -next] usb: gadget: dereference before null check

On Wed, Jun 29, 2022 at 05:44:55PM +0530, Sebin Sebastian wrote:
> On Wed, Jun 29, 2022 at 10:24:07AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Jun 29, 2022 at 01:37:25PM +0530, SebinSebastian wrote:
> > > Fix coverity warning dereferencing before null check. _ep and desc is
> > > dereferenced on all paths until the check for null. Move the
> > > initializations after the check for null.
> > > Coverity issue: 1518209
> > >
> > > Signed-off-by: SebinSebastian <[email protected]>
> > > ---
> > > drivers/usb/gadget/udc/aspeed_udc.c | 9 +++++----
> > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c
> > > index d75a4e070bf7..96f8193fca15 100644
> > > --- a/drivers/usb/gadget/udc/aspeed_udc.c
> > > +++ b/drivers/usb/gadget/udc/aspeed_udc.c
> > > @@ -341,10 +341,6 @@ static void ast_udc_stop_activity(struct ast_udc_dev *udc)
> > > static int ast_udc_ep_enable(struct usb_ep *_ep,
> > > const struct usb_endpoint_descriptor *desc)
> > > {
> > > - u16 maxpacket = usb_endpoint_maxp(desc);
> > > - struct ast_udc_ep *ep = to_ast_ep(_ep);
> > > - struct ast_udc_dev *udc = ep->udc;
> > > - u8 epnum = usb_endpoint_num(desc);
> > > unsigned long flags;
> > > u32 ep_conf = 0;
> > > u8 dir_in;
> > > @@ -356,6 +352,11 @@ static int ast_udc_ep_enable(struct usb_ep *_ep,
> > > return -EINVAL;
> > > }
> > >
> > > + u16 maxpacket = usb_endpoint_maxp(desc);
> > > + struct ast_udc_ep *ep = to_ast_ep(_ep);
> > > + struct ast_udc_dev *udc = ep->udc;
> > > + u8 epnum = usb_endpoint_num(desc);
> > > +
> > > if (!udc->driver) {
> > > EP_DBG(ep, "bogus device state\n");
> > > return -ESHUTDOWN;
> > > --
> > > 2.34.1
> > >
> >
> > Hi,
> >
> > This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
> > a patch that has triggered this response. He used to manually respond
> > to these common problems, but in order to save his sanity (he kept
> > writing the same thing over and over, yet to different people), I was
> > created. Hopefully you will not take offence and will fix the problem
> > in your patch and resubmit it so that it can be accepted into the Linux
> > kernel tree.
> >
> > You are receiving this message because of the following common error(s)
> > as indicated below:
> >
> > - Your patch breaks the build.
> >
> > - Your patch contains warnings and/or errors noticed by the
> > scripts/checkpatch.pl tool.
> >
> > - This looks like a new version of a previously submitted patch, but you
> > did not list below the --- line any changes from the previous version.
> > Please read the section entitled "The canonical patch format" in the
> > kernel file, Documentation/SubmittingPatches for what needs to be done
> > here to properly describe this.
> >
> > If you wish to discuss this problem further, or you have questions about
> > how to resolve this issue, please feel free to respond to this email and
> > Greg will reply once he has dug out from the pending patches received
> > from other developers.
> >
> > thanks,
> >
> > greg k-h's patch email bot
>
> I am sorry to keep on bothering with this incorrect patches. I am
> running the checkpatch script everytime before I sent any patches. It is
> not showing any warnings or errors. Is it because of my name that my
> patches are getting rejected? I can see a space missing.

Did you test build your patch? If not, why not?

thanks,

greg k-h

2022-06-29 18:03:56

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH -next] usb: gadget: dereference before null check

Hi SebinSebastian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20220628]

url: https://github.com/intel-lab-lkp/linux/commits/SebinSebastian/usb-gadget-dereference-before-null-check/20220629-161008
base: cb71b93c2dc36d18a8b05245973328d018272cdf
config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20220630/[email protected]/config)
compiler: mips-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/97ebbd93f269a58b3b5a003898d6e09c29a73ab0
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review SebinSebastian/usb-gadget-dereference-before-null-check/20220629-161008
git checkout 97ebbd93f269a58b3b5a003898d6e09c29a73ab0
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/usb/gadget/udc/aspeed_udc.c: In function 'ast_udc_ep_enable':
>> drivers/usb/gadget/udc/aspeed_udc.c:349:22: error: 'ep' undeclared (first use in this function); did you mean '_ep'?
349 | if (!_ep || !ep || !desc || desc->bDescriptorType != USB_DT_ENDPOINT ||
| ^~
| _ep
drivers/usb/gadget/udc/aspeed_udc.c:349:22: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/usb/gadget/udc/aspeed_udc.c:350:13: error: 'maxpacket' undeclared (first use in this function)
350 | maxpacket == 0 || maxpacket > ep->ep.maxpacket) {
| ^~~~~~~~~
drivers/usb/gadget/udc/aspeed_udc.c:355:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
355 | u16 maxpacket = usb_endpoint_maxp(desc);
| ^~~


vim +349 drivers/usb/gadget/udc/aspeed_udc.c

055276c1320564b Neal Liu 2022-05-23 340
055276c1320564b Neal Liu 2022-05-23 341 static int ast_udc_ep_enable(struct usb_ep *_ep,
055276c1320564b Neal Liu 2022-05-23 342 const struct usb_endpoint_descriptor *desc)
055276c1320564b Neal Liu 2022-05-23 343 {
055276c1320564b Neal Liu 2022-05-23 344 unsigned long flags;
055276c1320564b Neal Liu 2022-05-23 345 u32 ep_conf = 0;
055276c1320564b Neal Liu 2022-05-23 346 u8 dir_in;
055276c1320564b Neal Liu 2022-05-23 347 u8 type;
055276c1320564b Neal Liu 2022-05-23 348
055276c1320564b Neal Liu 2022-05-23 @349 if (!_ep || !ep || !desc || desc->bDescriptorType != USB_DT_ENDPOINT ||
055276c1320564b Neal Liu 2022-05-23 @350 maxpacket == 0 || maxpacket > ep->ep.maxpacket) {
055276c1320564b Neal Liu 2022-05-23 351 EP_DBG(ep, "Failed, invalid EP enable param\n");
055276c1320564b Neal Liu 2022-05-23 352 return -EINVAL;
055276c1320564b Neal Liu 2022-05-23 353 }
055276c1320564b Neal Liu 2022-05-23 354
97ebbd93f269a58 SebinSebastian 2022-06-29 355 u16 maxpacket = usb_endpoint_maxp(desc);
97ebbd93f269a58 SebinSebastian 2022-06-29 356 struct ast_udc_ep *ep = to_ast_ep(_ep);
97ebbd93f269a58 SebinSebastian 2022-06-29 357 struct ast_udc_dev *udc = ep->udc;
97ebbd93f269a58 SebinSebastian 2022-06-29 358 u8 epnum = usb_endpoint_num(desc);
97ebbd93f269a58 SebinSebastian 2022-06-29 359
055276c1320564b Neal Liu 2022-05-23 360 if (!udc->driver) {
055276c1320564b Neal Liu 2022-05-23 361 EP_DBG(ep, "bogus device state\n");
055276c1320564b Neal Liu 2022-05-23 362 return -ESHUTDOWN;
055276c1320564b Neal Liu 2022-05-23 363 }
055276c1320564b Neal Liu 2022-05-23 364
055276c1320564b Neal Liu 2022-05-23 365 EP_DBG(ep, "maxpacket:0x%x\n", maxpacket);
055276c1320564b Neal Liu 2022-05-23 366
055276c1320564b Neal Liu 2022-05-23 367 spin_lock_irqsave(&udc->lock, flags);
055276c1320564b Neal Liu 2022-05-23 368
055276c1320564b Neal Liu 2022-05-23 369 ep->desc = desc;
055276c1320564b Neal Liu 2022-05-23 370 ep->stopped = 0;
055276c1320564b Neal Liu 2022-05-23 371 ep->ep.maxpacket = maxpacket;
055276c1320564b Neal Liu 2022-05-23 372 ep->chunk_max = AST_EP_DMA_DESC_MAX_LEN;
055276c1320564b Neal Liu 2022-05-23 373
055276c1320564b Neal Liu 2022-05-23 374 if (maxpacket < AST_UDC_EPn_MAX_PACKET)
055276c1320564b Neal Liu 2022-05-23 375 ep_conf = EP_SET_MAX_PKT(maxpacket);
055276c1320564b Neal Liu 2022-05-23 376
055276c1320564b Neal Liu 2022-05-23 377 ep_conf |= EP_SET_EP_NUM(epnum);
055276c1320564b Neal Liu 2022-05-23 378
055276c1320564b Neal Liu 2022-05-23 379 type = usb_endpoint_type(desc);
055276c1320564b Neal Liu 2022-05-23 380 dir_in = usb_endpoint_dir_in(desc);
055276c1320564b Neal Liu 2022-05-23 381 ep->dir_in = dir_in;
055276c1320564b Neal Liu 2022-05-23 382 if (!ep->dir_in)
055276c1320564b Neal Liu 2022-05-23 383 ep_conf |= EP_DIR_OUT;
055276c1320564b Neal Liu 2022-05-23 384
055276c1320564b Neal Liu 2022-05-23 385 EP_DBG(ep, "type %d, dir_in %d\n", type, dir_in);
055276c1320564b Neal Liu 2022-05-23 386 switch (type) {
055276c1320564b Neal Liu 2022-05-23 387 case USB_ENDPOINT_XFER_ISOC:
055276c1320564b Neal Liu 2022-05-23 388 ep_conf |= EP_SET_TYPE_MASK(EP_TYPE_ISO);
055276c1320564b Neal Liu 2022-05-23 389 break;
055276c1320564b Neal Liu 2022-05-23 390
055276c1320564b Neal Liu 2022-05-23 391 case USB_ENDPOINT_XFER_BULK:
055276c1320564b Neal Liu 2022-05-23 392 ep_conf |= EP_SET_TYPE_MASK(EP_TYPE_BULK);
055276c1320564b Neal Liu 2022-05-23 393 break;
055276c1320564b Neal Liu 2022-05-23 394
055276c1320564b Neal Liu 2022-05-23 395 case USB_ENDPOINT_XFER_INT:
055276c1320564b Neal Liu 2022-05-23 396 ep_conf |= EP_SET_TYPE_MASK(EP_TYPE_INT);
055276c1320564b Neal Liu 2022-05-23 397 break;
055276c1320564b Neal Liu 2022-05-23 398 }
055276c1320564b Neal Liu 2022-05-23 399
055276c1320564b Neal Liu 2022-05-23 400 ep->desc_mode = udc->desc_mode && ep->descs_dma && ep->dir_in;
055276c1320564b Neal Liu 2022-05-23 401 if (ep->desc_mode) {
055276c1320564b Neal Liu 2022-05-23 402 ast_ep_write(ep, EP_DMA_CTRL_RESET, AST_UDC_EP_DMA_CTRL);
055276c1320564b Neal Liu 2022-05-23 403 ast_ep_write(ep, 0, AST_UDC_EP_DMA_STS);
055276c1320564b Neal Liu 2022-05-23 404 ast_ep_write(ep, ep->descs_dma, AST_UDC_EP_DMA_BUFF);
055276c1320564b Neal Liu 2022-05-23 405
055276c1320564b Neal Liu 2022-05-23 406 /* Enable Long Descriptor Mode */
055276c1320564b Neal Liu 2022-05-23 407 ast_ep_write(ep, EP_DMA_CTRL_IN_LONG_MODE | EP_DMA_DESC_MODE,
055276c1320564b Neal Liu 2022-05-23 408 AST_UDC_EP_DMA_CTRL);
055276c1320564b Neal Liu 2022-05-23 409
055276c1320564b Neal Liu 2022-05-23 410 ep->descs_wptr = 0;
055276c1320564b Neal Liu 2022-05-23 411
055276c1320564b Neal Liu 2022-05-23 412 } else {
055276c1320564b Neal Liu 2022-05-23 413 ast_ep_write(ep, EP_DMA_CTRL_RESET, AST_UDC_EP_DMA_CTRL);
055276c1320564b Neal Liu 2022-05-23 414 ast_ep_write(ep, EP_DMA_SINGLE_STAGE, AST_UDC_EP_DMA_CTRL);
055276c1320564b Neal Liu 2022-05-23 415 ast_ep_write(ep, 0, AST_UDC_EP_DMA_STS);
055276c1320564b Neal Liu 2022-05-23 416 }
055276c1320564b Neal Liu 2022-05-23 417
055276c1320564b Neal Liu 2022-05-23 418 /* Cleanup data toggle just in case */
055276c1320564b Neal Liu 2022-05-23 419 ast_udc_write(udc, EP_TOGGLE_SET_EPNUM(epnum), AST_VHUB_EP_DATA);
055276c1320564b Neal Liu 2022-05-23 420
055276c1320564b Neal Liu 2022-05-23 421 /* Enable EP */
055276c1320564b Neal Liu 2022-05-23 422 ast_ep_write(ep, ep_conf | EP_ENABLE, AST_UDC_EP_CONFIG);
055276c1320564b Neal Liu 2022-05-23 423
055276c1320564b Neal Liu 2022-05-23 424 EP_DBG(ep, "ep_config: 0x%x\n", ast_ep_read(ep, AST_UDC_EP_CONFIG));
055276c1320564b Neal Liu 2022-05-23 425
055276c1320564b Neal Liu 2022-05-23 426 spin_unlock_irqrestore(&udc->lock, flags);
055276c1320564b Neal Liu 2022-05-23 427
055276c1320564b Neal Liu 2022-05-23 428 return 0;
055276c1320564b Neal Liu 2022-05-23 429 }
055276c1320564b Neal Liu 2022-05-23 430

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-06-30 04:26:24

by Sebin Sebastian

[permalink] [raw]
Subject: Re: [PATCH -next] usb: gadget: dereference before null check

On Wed, Jun 29, 2022 at 05:31:57PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jun 29, 2022 at 05:44:55PM +0530, Sebin Sebastian wrote:
> > On Wed, Jun 29, 2022 at 10:24:07AM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, Jun 29, 2022 at 01:37:25PM +0530, SebinSebastian wrote:
> > > > Fix coverity warning dereferencing before null check. _ep and desc is
> > > > dereferenced on all paths until the check for null. Move the
> > > > initializations after the check for null.
> > > > Coverity issue: 1518209
> > > >
> > > > Signed-off-by: SebinSebastian <[email protected]>
> > > > ---
> > > > drivers/usb/gadget/udc/aspeed_udc.c | 9 +++++----
> > > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c
> > > > index d75a4e070bf7..96f8193fca15 100644
> > > > --- a/drivers/usb/gadget/udc/aspeed_udc.c
> > > > +++ b/drivers/usb/gadget/udc/aspeed_udc.c
> > > > @@ -341,10 +341,6 @@ static void ast_udc_stop_activity(struct ast_udc_dev *udc)
> > > > static int ast_udc_ep_enable(struct usb_ep *_ep,
> > > > const struct usb_endpoint_descriptor *desc)
> > > > {
> > > > - u16 maxpacket = usb_endpoint_maxp(desc);
> > > > - struct ast_udc_ep *ep = to_ast_ep(_ep);
> > > > - struct ast_udc_dev *udc = ep->udc;
> > > > - u8 epnum = usb_endpoint_num(desc);
> > > > unsigned long flags;
> > > > u32 ep_conf = 0;
> > > > u8 dir_in;
> > > > @@ -356,6 +352,11 @@ static int ast_udc_ep_enable(struct usb_ep *_ep,
> > > > return -EINVAL;
> > > > }
> > > >
> > > > + u16 maxpacket = usb_endpoint_maxp(desc);
> > > > + struct ast_udc_ep *ep = to_ast_ep(_ep);
> > > > + struct ast_udc_dev *udc = ep->udc;
> > > > + u8 epnum = usb_endpoint_num(desc);
> > > > +
> > > > if (!udc->driver) {
> > > > EP_DBG(ep, "bogus device state\n");
> > > > return -ESHUTDOWN;
> > > > --
> > > > 2.34.1
> > > >
> > >
> > > Hi,
> > >
> > > This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
> > > a patch that has triggered this response. He used to manually respond
> > > to these common problems, but in order to save his sanity (he kept
> > > writing the same thing over and over, yet to different people), I was
> > > created. Hopefully you will not take offence and will fix the problem
> > > in your patch and resubmit it so that it can be accepted into the Linux
> > > kernel tree.
> > >
> > > You are receiving this message because of the following common error(s)
> > > as indicated below:
> > >
> > > - Your patch breaks the build.
> > >
> > > - Your patch contains warnings and/or errors noticed by the
> > > scripts/checkpatch.pl tool.
> > >
> > > - This looks like a new version of a previously submitted patch, but you
> > > did not list below the --- line any changes from the previous version.
> > > Please read the section entitled "The canonical patch format" in the
> > > kernel file, Documentation/SubmittingPatches for what needs to be done
> > > here to properly describe this.
> > >
> > > If you wish to discuss this problem further, or you have questions about
> > > how to resolve this issue, please feel free to respond to this email and
> > > Greg will reply once he has dug out from the pending patches received
> > > from other developers.
> > >
> > > thanks,
> > >
> > > greg k-h's patch email bot
> >
> > I am sorry to keep on bothering with this incorrect patches. I am
> > running the checkpatch script everytime before I sent any patches. It is
> > not showing any warnings or errors. Is it because of my name that my
> > patches are getting rejected? I can see a space missing.
>
> Did you test build your patch? If not, why not?
>
> thanks,
>
> greg k-h

Ok, now I understand the source of all errors. I did build the entire
tree, but make never touched udc. I have fixed all errors and warnings,
build the patch properly, ran through checkpatch and is now ready for
submission.