[PATCH] usb: dwc2: gadget: Fix a warning when compiling with W=1

[PATCH] usb: dwc2: gadget: Fix a warning when compiling with W=1

[Christophe JAILLET]

In order to teach the compiler that 'hs_ep->name' will never be truncated,
we need to tell it that 'epnum' is not negative.

'epnum' comes from in a 'for' loop in dwc2_gadget_init(), starting at 0
and ending at 255. (hsotg->num_of_eps is a char)

When building with W=1, this fixes the following warnings:

  drivers/usb/dwc2/gadget.c: In function ‘dwc2_hsotg_initep’:
  drivers/usb/dwc2/gadget.c:4804:55: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 8 [-Werror=format-truncation=]
   4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%d%s", epnum, dir);
        |                                                       ^~
  drivers/usb/dwc2/gadget.c:4804:52: note: directive argument in the range [-2147483645, 255]
   4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%d%s", epnum, dir);
        |                                                    ^~~~~~~~
  drivers/usb/dwc2/gadget.c:4804:9: note: ‘snprintf’ output between 6 and 17 bytes into a destination of size 10
   4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%d%s", epnum, dir);
        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 5b7d70c6dbf2 ("USB: Gadget driver for Samsung HS/OtG block")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Only changing:
  -	int epnum;
  +	unsigned int epnum;
is enought to fix the build warning.

But changing the prototype of dwc2_hsotg_initep() and the printf() format
as well, to make obvious that epnum is >= 0, looks more logical to me.
---
 drivers/usb/dwc2/gadget.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index b517a7216de2..102b2dd8113e 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4786,8 +4786,8 @@ static const struct usb_gadget_ops dwc2_hsotg_gadget_ops = {
  */
 static void dwc2_hsotg_initep(struct dwc2_hsotg *hsotg,
 			      struct dwc2_hsotg_ep *hs_ep,
-				       int epnum,
-				       bool dir_in)
+			      unsigned int epnum,
+			      bool dir_in)
 {
 	char *dir;
 
@@ -4801,7 +4801,7 @@ static void dwc2_hsotg_initep(struct dwc2_hsotg *hsotg,
 	hs_ep->dir_in = dir_in;
 	hs_ep->index = epnum;
 
-	snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%d%s", epnum, dir);
+	snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%u%s", epnum, dir);
 
 	INIT_LIST_HEAD(&hs_ep->queue);
 	INIT_LIST_HEAD(&hs_ep->ep.ep_list);
@@ -4965,7 +4965,7 @@ static void dwc2_hsotg_dump(struct dwc2_hsotg *hsotg)
 int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
 {
 	struct device *dev = hsotg->dev;
-	int epnum;
+	unsigned int epnum;
 	int ret;
 
 	/* Dump fifo information */
-- 
2.34.1

Re: [PATCH] usb: dwc2: gadget: Fix a warning when compiling with W=1

[kernel test robot]

Hi Christophe,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.6-rc3 next-20230926]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christophe-JAILLET/usb-dwc2-gadget-Fix-a-warning-when-compiling-with-W-1/20230923-185559
base:   linus/master
patch link:    https://lore.kernel.org/r/5cf603809388aa04c9a02bbfe3cf531c20bb043e.1695466447.git.christophe.jaillet%40wanadoo.fr
patch subject: [PATCH] usb: dwc2: gadget: Fix a warning when compiling with W=1
config: x86_64-buildonly-randconfig-004-20230927 (https://download.01.org/0day-ci/archive/20230927/202309270325.uqGsh5Cw-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230927/202309270325.uqGsh5Cw-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309270325.uqGsh5Cw-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/usb/dwc2/gadget.c: In function 'dwc2_hsotg_initep':
>> drivers/usb/dwc2/gadget.c:4804:55: warning: '%u' directive output may be truncated writing between 1 and 10 bytes into a region of size 8 [-Wformat-truncation=]
    4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%u%s", epnum, dir);
         |                                                       ^~
   drivers/usb/dwc2/gadget.c:4804:52: note: directive argument in the range [1, 4294967295]
    4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%u%s", epnum, dir);
         |                                                    ^~~~~~~~
   drivers/usb/dwc2/gadget.c:4804:9: note: 'snprintf' output between 6 and 16 bytes into a destination of size 10
    4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%u%s", epnum, dir);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +4804 drivers/usb/dwc2/gadget.c

  4775	
  4776	/**
  4777	 * dwc2_hsotg_initep - initialise a single endpoint
  4778	 * @hsotg: The device state.
  4779	 * @hs_ep: The endpoint to be initialised.
  4780	 * @epnum: The endpoint number
  4781	 * @dir_in: True if direction is in.
  4782	 *
  4783	 * Initialise the given endpoint (as part of the probe and device state
  4784	 * creation) to give to the gadget driver. Setup the endpoint name, any
  4785	 * direction information and other state that may be required.
  4786	 */
  4787	static void dwc2_hsotg_initep(struct dwc2_hsotg *hsotg,
  4788				      struct dwc2_hsotg_ep *hs_ep,
  4789				      unsigned int epnum,
  4790				      bool dir_in)
  4791	{
  4792		char *dir;
  4793	
  4794		if (epnum == 0)
  4795			dir = "";
  4796		else if (dir_in)
  4797			dir = "in";
  4798		else
  4799			dir = "out";
  4800	
  4801		hs_ep->dir_in = dir_in;
  4802		hs_ep->index = epnum;
  4803	
> 4804		snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%u%s", epnum, dir);
  4805	
  4806		INIT_LIST_HEAD(&hs_ep->queue);
  4807		INIT_LIST_HEAD(&hs_ep->ep.ep_list);
  4808	
  4809		/* add to the list of endpoints known by the gadget driver */
  4810		if (epnum)
  4811			list_add_tail(&hs_ep->ep.ep_list, &hsotg->gadget.ep_list);
  4812	
  4813		hs_ep->parent = hsotg;
  4814		hs_ep->ep.name = hs_ep->name;
  4815	
  4816		if (hsotg->params.speed == DWC2_SPEED_PARAM_LOW)
  4817			usb_ep_set_maxpacket_limit(&hs_ep->ep, 8);
  4818		else
  4819			usb_ep_set_maxpacket_limit(&hs_ep->ep,
  4820						   epnum ? 1024 : EP0_MPS_LIMIT);
  4821		hs_ep->ep.ops = &dwc2_hsotg_ep_ops;
  4822	
  4823		if (epnum == 0) {
  4824			hs_ep->ep.caps.type_control = true;
  4825		} else {
  4826			if (hsotg->params.speed != DWC2_SPEED_PARAM_LOW) {
  4827				hs_ep->ep.caps.type_iso = true;
  4828				hs_ep->ep.caps.type_bulk = true;
  4829			}
  4830			hs_ep->ep.caps.type_int = true;
  4831		}
  4832	
  4833		if (dir_in)
  4834			hs_ep->ep.caps.dir_in = true;
  4835		else
  4836			hs_ep->ep.caps.dir_out = true;
  4837	
  4838		/*
  4839		 * if we're using dma, we need to set the next-endpoint pointer
  4840		 * to be something valid.
  4841		 */
  4842	
  4843		if (using_dma(hsotg)) {
  4844			u32 next = DXEPCTL_NEXTEP((epnum + 1) % 15);
  4845	
  4846			if (dir_in)
  4847				dwc2_writel(hsotg, next, DIEPCTL(epnum));
  4848			else
  4849				dwc2_writel(hsotg, next, DOEPCTL(epnum));
  4850		}
  4851	}
  4852	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] usb: dwc2: gadget: Fix a warning when compiling with W=1

[Greg Kroah-Hartman]

On Sat, Sep 23, 2023 at 12:54:24PM +0200, Christophe JAILLET wrote:
> In order to teach the compiler that 'hs_ep->name' will never be truncated,
> we need to tell it that 'epnum' is not negative.
> 
> 'epnum' comes from in a 'for' loop in dwc2_gadget_init(), starting at 0
> and ending at 255. (hsotg->num_of_eps is a char)
> 
> When building with W=1, this fixes the following warnings:
> 
>   drivers/usb/dwc2/gadget.c: In function ‘dwc2_hsotg_initep’:
>   drivers/usb/dwc2/gadget.c:4804:55: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 8 [-Werror=format-truncation=]
>    4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%d%s", epnum, dir);
>         |                                                       ^~
>   drivers/usb/dwc2/gadget.c:4804:52: note: directive argument in the range [-2147483645, 255]
>    4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%d%s", epnum, dir);
>         |                                                    ^~~~~~~~
>   drivers/usb/dwc2/gadget.c:4804:9: note: ‘snprintf’ output between 6 and 17 bytes into a destination of size 10
>    4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%d%s", epnum, dir);
>         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fixes: 5b7d70c6dbf2 ("USB: Gadget driver for Samsung HS/OtG block")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Looks like the kernel test robot didn't like this one :(

Re: [PATCH] usb: dwc2: gadget: Fix a warning when compiling with W=1

[Christophe JAILLET]

Le 02/10/2023 à 13:45, Greg Kroah-Hartman a écrit :
> On Sat, Sep 23, 2023 at 12:54:24PM +0200, Christophe JAILLET wrote:
>> In order to teach the compiler that 'hs_ep->name' will never be truncated,
>> we need to tell it that 'epnum' is not negative.
>>
>> 'epnum' comes from in a 'for' loop in dwc2_gadget_init(), starting at 0
>> and ending at 255. (hsotg->num_of_eps is a char)
>>
>> When building with W=1, this fixes the following warnings:
>>
>>    drivers/usb/dwc2/gadget.c: In function ‘dwc2_hsotg_initep’:
>>    drivers/usb/dwc2/gadget.c:4804:55: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 8 [-Werror=format-truncation=]
>>     4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%d%s", epnum, dir);
>>          |                                                       ^~
>>    drivers/usb/dwc2/gadget.c:4804:52: note: directive argument in the range [-2147483645, 255]
>>     4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%d%s", epnum, dir);
>>          |                                                    ^~~~~~~~
>>    drivers/usb/dwc2/gadget.c:4804:9: note: ‘snprintf’ output between 6 and 17 bytes into a destination of size 10
>>     4804 |         snprintf(hs_ep->name, sizeof(hs_ep->name), "ep%d%s", epnum, dir);
>>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Fixes: 5b7d70c6dbf2 ("USB: Gadget driver for Samsung HS/OtG block")
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> 
> Looks like the kernel test robot didn't like this one :(
> 

Hi,

unless I missed something, this was built-tested.
I use gcc 12.3.0 and the report is done with gcc 11.3.0.

Maybe the value range propagation algorithm of how the diagnostic for 
such potential overflow has been improved in recent gcc?


For your information, I got a similar feedback for another patch.
It was also built tested from my side, but the maintainer report that 
there is still some potential overflow warning.

Strange :/

CJ