drivers/usb/dwc2/gadget.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
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
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
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
© 2016 - 2026 Red Hat, Inc.