usb: isp1760: increase buffer size to avoid overflow

PATCH In review Alexey V. Vissarionov 4 participants 6 messages Apr 22, 2025 – Apr 24, 2025

Subsystems (2)

USB USB ISP1760 driver

lore

Share & tools

lore

Alexey V. Vissarionov

author

2025-04-22 23:00 UTC #0 lore raw

PATCH 1/1 usb: isp1760: increase buffer size to avoid overflow

isp1760_field_set() may access the udc->fields array beyond the size of DC_FIELD_MAX up to HC_FIELD_MAX, which is (now) bigger. Increase the buffer size to max(DC_FIELD_MAX,HC_FIELD_MAX) to avoid possible overflow.

Found by ALT Linux Team (altlinux.org) and Linux Verification Center (linuxtesting.org).

Fixes 1da9e1c06873 ("usb: isp1760: move to regmap for register access")

Signed-off-by Alexey V. Vissarionov <[email protected]>

---
 drivers/usb/isp1760/isp1760-hcd.h  | 2 +-
 drivers/usb/isp1760/isp1760-regs.h | 2 ++
 drivers/usb/isp1760/isp1760-udc.h  | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

drivers/usb/isp1760/isp1760-hcd.h +1 -1

index ee3063a34de3bccf..a2ba48c84a8dd513 100644
@@ -50,7 +50,7 @@ struct isp1760_hcd {
 	void __iomem		*base;
 
 	struct regmap		*regs;
-	struct regmap_field	*fields[HC_FIELD_MAX];
+	struct regmap_field	*fields[FIELD_MAX];
 
 	bool			is_isp1763;
 	const struct isp1760_memory_layout	*memory_layout;

drivers/usb/isp1760/isp1760-regs.h +2 -0

index 3a6751197e970013..b4644fc1f88a82f0 100644
@@ -267,6 +267,8 @@ enum isp176x_device_controller_fields {
 	DC_FIELD_MAX,
 };
 
+#define	FIELD_MAX	(DC_FIELD_MAX>HC_FIELD_MAX?DC_FIELD_MAX:HC_FIELD_MAX)
+
 /* ISP1763 */
 /* Initialization Registers */
 #define ISP1763_DC_ADDRESS		0x00

drivers/usb/isp1760/isp1760-udc.h +1 -1

index 22044e86bc0ecb84..e01c95161526a3db 100644
@@ -69,7 +69,7 @@ struct isp1760_udc {
 	char *irqname;
 
 	struct regmap *regs;
-	struct regmap_field *fields[DC_FIELD_MAX];
+	struct regmap_field *fields[FIELD_MAX];
 
 	struct usb_gadget_driver *driver;
 	struct usb_gadget gadget;

Signature · 3 lines

Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net

Fedor Pchelkin

2025-04-23 06:30 UTC #1 lore raw

Quoted 4 lines · Alexey V. Vissarionov

isp1760_field_set() may access the udc->fields array beyond the size of DC_FIELD_MAX up to HC_FIELD_MAX, which is (now) bigger. Increase the buffer size to max(DC_FIELD_MAX,HC_FIELD_MAX) to avoid possible overflow.

Where exactly does the problem manifest? There is no comprehensible bug description here from you..

Though I guess isp1760_set_pullup() call site is concerned?

drivers/usb/isp1760/isp1760-regs.h enum isp176x_device_controller_fields { · line 271

Quoted 6 lines

drivers/usb/isp1760/isp1760-regs.h +2 -0

@@ -267,6 +267,8 @@ enum isp176x_device_controller_fields {
 	DC_FIELD_MAX,
 };
 
+#define	FIELD_MAX	(DC_FIELD_MAX>HC_FIELD_MAX?DC_FIELD_MAX:HC_FIELD_MAX)
+

Please make sure to run your changes through checkpatch.pl and the compiler first. They are both not happy at the moment.

In file included from drivers/usb/isp1760/isp1760-hcd.h:8,

                 from drivers/usb/isp1760/isp1760-core.h:21,
                 from drivers/usb/isp1760/isp1760-core.c:24:
drivers/usb/isp1760/isp1760-regs.h:270:9: warning: ‘FIELD_MAX’ redefined
  270 | #define FIELD_MAX       (DC_FIELD_MAX>HC_FIELD_MAX?DC_FIELD_MAX:HC_FIELD_MAX)
      |         ^~~~~~~~~
In file included from ./include/linux/fortify-string.h:5,
                 from ./include/linux/string.h:392,
                 from ./include/linux/bitmap.h:13,
                 from ./include/linux/cpumask.h:12,
                 from ./arch/x86/include/asm/tlbbatch.h:5,
                 from ./include/linux/mm_types_task.h:17,
                 from ./include/linux/sched.h:38,
                 from ./include/linux/delay.h:13,
                 from drivers/usb/isp1760/isp1760-core.c:15:
./include/linux/bitfield.h:86:9: note: this is the location of the previous definition
   86 | #define FIELD_MAX(_mask)                                                \
      |         ^~~~~~~~~

drivers/usb/isp1760/isp1760-regs.h:270:38: warning: comparison between ‘enum isp176x_device_controller_fields’ and ‘enum isp176x_host_controller_fields’ [-Wenum-compare]
270 | #define FIELD_MAX (DC_FIELD_MAX>HC_FIELD_MAX?DC_FIELD_MAX:HC_FIELD_MAX)
| ^
drivers/usb/isp1760/isp1760-hcd.h:53:41: note: in expansion of macro ‘FIELD_MAX’
53 | struct regmap_field *fields[FIELD_MAX];
| ^~~~~~~~~
drivers/usb/isp1760/isp1760-regs.h:270:38: warning: comparison between ‘enum isp176x_device_controller_fields’ and ‘enum isp176x_host_controller_fields’ [-Wenum-compare]
270 | #define FIELD_MAX (DC_FIELD_MAX>HC_FIELD_MAX?DC_FIELD_MAX:HC_FIELD_MAX)
| ^
drivers/usb/isp1760/isp1760-udc.h:72:37: note: in expansion of macro ‘FIELD_MAX’
72 | struct regmap_field *fields[FIELD_MAX];
| ^~~~~~~~~

Thanks!

Alexey V. Vissarionov

author

2025-04-23 11:05 UTC #2 lore raw

Good ${greeting_time}!

Quoted 7 lines · Fedor Pchelkin

Quoted 3 lines · Alexey V. Vissarionov
isp1760_field_set() may access the udc->fields array beyond the size of DC_FIELD_MAX up to HC_FIELD_MAX, which is (now) bigger.
Where exactly does the problem manifest? There is no comprehensible bug description here from you.. Though I guess isp1760_set_pullup() call site is concerned?

Yes, when called from isp1760_udc_init_hw()

+ When isp1760_udc_init_hw() calls isp1760_set_pullup(),

Quoted 2 lines

Please make sure to run your changes through checkpatch.pl and the compiler first.

Mea culpa... Sent earlier version.

There should be only one warning on "Comparisons should place the constant on the right side of the test" when comparing two constants.

Updated patch follows.

Signature · 3 lines

Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net

Alexey V. Vissarionov

author

2025-04-23 11:10 UTC #3 lore raw

PATCH 1/1 usb: isp1760: increase buffer size to avoid overflow

When isp1760_udc_init_hw() calls isp1760_set_pullup(), its call of isp1760_field_set() may access the udc->fields array beyond the size of DC_FIELD_MAX up to HC_FIELD_MAX, which is (now) bigger. Increase the buffer size to max(DC_FIELD_MAX,HC_FIELD_MAX) to avoid possible overflow.

Found by ALT Linux Team (altlinux.org) and Linux Verification Center (linuxtesting.org).

Fixes 1da9e1c06873 ("usb: isp1760: move to regmap for register access")

Signed-off-by Alexey V. Vissarionov <[email protected]>

---
 drivers/usb/isp1760/isp1760-hcd.h  | 2 +-
 drivers/usb/isp1760/isp1760-regs.h | 3 +++
 drivers/usb/isp1760/isp1760-udc.h  | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

drivers/usb/isp1760/isp1760-hcd.h +1 -1

index ee3063a34de3bccf..34dacde96c4ae3cf 100644
@@ -50,7 +50,7 @@ struct isp1760_hcd {
 	void __iomem		*base;
 
 	struct regmap		*regs;
-	struct regmap_field	*fields[HC_FIELD_MAX];
+	struct regmap_field	*fields[DC_HC_FIELD_MAX];
 
 	bool			is_isp1763;
 	const struct isp1760_memory_layout	*memory_layout;

drivers/usb/isp1760/isp1760-regs.h +3 -0

index 3a6751197e970013..a5a442015887ce0b 100644
@@ -267,6 +267,9 @@ enum isp176x_device_controller_fields {
 	DC_FIELD_MAX,
 };
 
+#define	DC_HC_FIELD_MAX	\
+	(DC_FIELD_MAX > HC_FIELD_MAX ? DC_FIELD_MAX : HC_FIELD_MAX)
+
 /* ISP1763 */
 /* Initialization Registers */
 #define ISP1763_DC_ADDRESS		0x00

drivers/usb/isp1760/isp1760-udc.h +1 -1

index 22044e86bc0ecb84..609444bea306ba81 100644
@@ -69,7 +69,7 @@ struct isp1760_udc {
 	char *irqname;
 
 	struct regmap *regs;
-	struct regmap_field *fields[DC_FIELD_MAX];
+	struct regmap_field *fields[DC_HC_FIELD_MAX];
 
 	struct usb_gadget_driver *driver;
 	struct usb_gadget gadget;

Signature · 3 lines

Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net

Rui Miguel Silva

2025-04-24 09:16 UTC #4 lore raw

Hey Alexey,

Quoted 5 lines · Alexey V. Vissarionov

When isp1760_udc_init_hw() calls isp1760_set_pullup(), its call of isp1760_field_set() may access the udc->fields array beyond the size of DC_FIELD_MAX up to HC_FIELD_MAX, which is (now) bigger. Increase the buffer size to max(DC_FIELD_MAX,HC_FIELD_MAX) to avoid possible overflow.

This will fix the access, but not the main issue, so this is not correct. The isp1760_set_pullup should pass to isp1760_field_set the hcd fields and not the udc ones.

I will send a proper fix for this. Thanks for reporting.

Cheers,
Rui

Quoted 57 lines

Found by ALT Linux Team (altlinux.org) and Linux Verification Center (linuxtesting.org).

Fixes 1da9e1c06873 ("usb: isp1760: move to regmap for register access")

Signed-off-by Alexey V. Vissarionov <[email protected]>

---
 drivers/usb/isp1760/isp1760-hcd.h  | 2 +-
 drivers/usb/isp1760/isp1760-regs.h | 3 +++
 drivers/usb/isp1760/isp1760-udc.h  | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

drivers/usb/isp1760/isp1760-hcd.h +1 -1

index ee3063a34de3bccf..34dacde96c4ae3cf 100644
@@ -50,7 +50,7 @@ struct isp1760_hcd {
 	void __iomem		*base;
 
 	struct regmap		*regs;
-	struct regmap_field	*fields[HC_FIELD_MAX];
+	struct regmap_field	*fields[DC_HC_FIELD_MAX];
 
 	bool			is_isp1763;
 	const struct isp1760_memory_layout	*memory_layout;

drivers/usb/isp1760/isp1760-regs.h +3 -0

index 3a6751197e970013..a5a442015887ce0b 100644
@@ -267,6 +267,9 @@ enum isp176x_device_controller_fields {
 	DC_FIELD_MAX,
 };
 
+#define	DC_HC_FIELD_MAX	\
+	(DC_FIELD_MAX > HC_FIELD_MAX ? DC_FIELD_MAX : HC_FIELD_MAX)
+
 /* ISP1763 */
 /* Initialization Registers */
 #define ISP1763_DC_ADDRESS		0x00

drivers/usb/isp1760/isp1760-udc.h +1 -1

index 22044e86bc0ecb84..609444bea306ba81 100644
@@ -69,7 +69,7 @@ struct isp1760_udc {
 	char *irqname;
 
 	struct regmap *regs;
-	struct regmap_field *fields[DC_FIELD_MAX];
+	struct regmap_field *fields[DC_HC_FIELD_MAX];
 
 	struct usb_gadget_driver *driver;
 	struct usb_gadget gadget;

Signature · 4 lines

-- 
Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net

kernel test robot

build bot

2025-04-24 10:04 UTC #5 lore raw

In reply to Alexey V. Vissarionov

kernel test robot noticed the following build warnings:

Hi Alexey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.15-rc3 next-20250424] [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/Alexey-V-Vissarionov/usb-isp1760-increase-buffer-size-to-avoid-overflow/20250423-191222
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20250423111040.GC857%40altlinux.org
patch subject: [PATCH v1] usb: isp1760: increase buffer size to avoid overflow
config: i386-buildonly-randconfig-001-20250424 (https://download.01.org/0day-ci/archive/20250424/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250424/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from drivers/usb/isp1760/isp1760-hcd.h:8,

                    from drivers/usb/isp1760/isp1760-core.h:21,
                    from drivers/usb/isp1760/isp1760-udc.c:22:
>> drivers/usb/isp1760/isp1760-regs.h:271:23: warning: comparison between 'enum isp176x_device_controller_fields' and 'enum isp176x_host_controller_fields' [-Wenum-compare]
     271 |         (DC_FIELD_MAX > HC_FIELD_MAX ? DC_FIELD_MAX : HC_FIELD_MAX)
         |                       ^
   drivers/usb/isp1760/isp1760-hcd.h:53:41: note: in expansion of macro 'DC_HC_FIELD_MAX'
      53 |         struct regmap_field     *fields[DC_HC_FIELD_MAX];
         |                                         ^~~~~~~~~~~~~~~
>> drivers/usb/isp1760/isp1760-regs.h:271:23: warning: comparison between 'enum isp176x_device_controller_fields' and 'enum isp176x_host_controller_fields' [-Wenum-compare]
     271 |         (DC_FIELD_MAX > HC_FIELD_MAX ? DC_FIELD_MAX : HC_FIELD_MAX)
         |                       ^
   drivers/usb/isp1760/isp1760-udc.h:72:37: note: in expansion of macro 'DC_HC_FIELD_MAX'
      72 |         struct regmap_field *fields[DC_HC_FIELD_MAX];
         |                                     ^~~~~~~~~~~~~~~

vim +271 drivers/usb/isp1760/isp1760-regs.h

269
270 #define DC_HC_FIELD_MAX \

 > 271		(DC_FIELD_MAX > HC_FIELD_MAX ? DC_FIELD_MAX : HC_FIELD_MAX)

272

Signature · 2 lines

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