| Message ID | 5cf603809388aa04c9a02bbfe3cf531c20bb043e.1695466447.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: dwc2: gadget: Fix a warning when compiling with W=1 | expand |
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
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 :(
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
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 */
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(-)