* [PATCH] usb: isp1760: increase buffer size to avoid overflow
@ 2025-04-22 23:00 Alexey V. Vissarionov
2025-04-23 6:30 ` Fedor Pchelkin
0 siblings, 1 reply; 6+ messages in thread
From: Alexey V. Vissarionov @ 2025-04-22 23:00 UTC (permalink / raw)
To: Rui Miguel Silva
Cc: Alexey V. Vissarionov, Greg Kroah-Hartman, linux-usb, lvc-project
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 <gremlin@altlinux.org>
---
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(-)
diff --git a/drivers/usb/isp1760/isp1760-hcd.h b/drivers/usb/isp1760/isp1760-hcd.h
index ee3063a34de3bccf..a2ba48c84a8dd513 100644
--- a/drivers/usb/isp1760/isp1760-hcd.h
+++ b/drivers/usb/isp1760/isp1760-hcd.h
@@ -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;
diff --git a/drivers/usb/isp1760/isp1760-regs.h b/drivers/usb/isp1760/isp1760-regs.h
index 3a6751197e970013..b4644fc1f88a82f0 100644
--- a/drivers/usb/isp1760/isp1760-regs.h
+++ b/drivers/usb/isp1760/isp1760-regs.h
@@ -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
diff --git a/drivers/usb/isp1760/isp1760-udc.h b/drivers/usb/isp1760/isp1760-udc.h
index 22044e86bc0ecb84..e01c95161526a3db 100644
--- a/drivers/usb/isp1760/isp1760-udc.h
+++ b/drivers/usb/isp1760/isp1760-udc.h
@@ -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;
--
Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: isp1760: increase buffer size to avoid overflow
2025-04-22 23:00 [PATCH] usb: isp1760: increase buffer size to avoid overflow Alexey V. Vissarionov
@ 2025-04-23 6:30 ` Fedor Pchelkin
2025-04-23 11:05 ` Alexey V. Vissarionov
0 siblings, 1 reply; 6+ messages in thread
From: Fedor Pchelkin @ 2025-04-23 6:30 UTC (permalink / raw)
To: Alexey V. Vissarionov
Cc: Rui Miguel Silva, Greg Kroah-Hartman, linux-usb, lvc-project
On Wed, 23. Apr 02:00, Alexey V. Vissarionov wrote:
> 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?
> @@ -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!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: isp1760: increase buffer size to avoid overflow
2025-04-23 6:30 ` Fedor Pchelkin
@ 2025-04-23 11:05 ` Alexey V. Vissarionov
2025-04-23 11:10 ` [PATCH v1] " Alexey V. Vissarionov
0 siblings, 1 reply; 6+ messages in thread
From: Alexey V. Vissarionov @ 2025-04-23 11:05 UTC (permalink / raw)
To: Fedor Pchelkin
Cc: Alexey V. Vissarionov, Rui Miguel Silva, Greg Kroah-Hartman,
linux-usb, lvc-project
Good ${greeting_time}!
On 2025-04-23 09:30:48 +0300, Fedor Pchelkin wrote:
> On Wed, 23. Apr 02:00, Alexey V. Vissarionov wrote:
>> 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(),
> 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.
--
Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1] usb: isp1760: increase buffer size to avoid overflow
2025-04-23 11:05 ` Alexey V. Vissarionov
@ 2025-04-23 11:10 ` Alexey V. Vissarionov
2025-04-24 9:15 ` Rui Miguel Silva
2025-04-24 10:04 ` kernel test robot
0 siblings, 2 replies; 6+ messages in thread
From: Alexey V. Vissarionov @ 2025-04-23 11:10 UTC (permalink / raw)
To: Rui Miguel Silva
Cc: Alexey V. Vissarionov, Fedor Pchelkin, Greg Kroah-Hartman,
linux-usb, lvc-project
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 <gremlin@altlinux.org>
---
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(-)
diff --git a/drivers/usb/isp1760/isp1760-hcd.h b/drivers/usb/isp1760/isp1760-hcd.h
index ee3063a34de3bccf..34dacde96c4ae3cf 100644
--- a/drivers/usb/isp1760/isp1760-hcd.h
+++ b/drivers/usb/isp1760/isp1760-hcd.h
@@ -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;
diff --git a/drivers/usb/isp1760/isp1760-regs.h b/drivers/usb/isp1760/isp1760-regs.h
index 3a6751197e970013..a5a442015887ce0b 100644
--- a/drivers/usb/isp1760/isp1760-regs.h
+++ b/drivers/usb/isp1760/isp1760-regs.h
@@ -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
diff --git a/drivers/usb/isp1760/isp1760-udc.h b/drivers/usb/isp1760/isp1760-udc.h
index 22044e86bc0ecb84..609444bea306ba81 100644
--- a/drivers/usb/isp1760/isp1760-udc.h
+++ b/drivers/usb/isp1760/isp1760-udc.h
@@ -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;
--
Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1] usb: isp1760: increase buffer size to avoid overflow
2025-04-23 11:10 ` [PATCH v1] " Alexey V. Vissarionov
@ 2025-04-24 9:15 ` Rui Miguel Silva
2025-04-24 10:04 ` kernel test robot
1 sibling, 0 replies; 6+ messages in thread
From: Rui Miguel Silva @ 2025-04-24 9:15 UTC (permalink / raw)
To: Alexey V. Vissarionov, Rui Miguel Silva
Cc: Fedor Pchelkin, Greg Kroah-Hartman, linux-usb, lvc-project
Hey Alexey,
On Wed Apr 23, 2025 at 12:10 PM WEST, Alexey V. Vissarionov wrote:
> 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
>
> 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 <gremlin@altlinux.org>
> ---
> 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(-)
>
> diff --git a/drivers/usb/isp1760/isp1760-hcd.h b/drivers/usb/isp1760/isp1760-hcd.h
> index ee3063a34de3bccf..34dacde96c4ae3cf 100644
> --- a/drivers/usb/isp1760/isp1760-hcd.h
> +++ b/drivers/usb/isp1760/isp1760-hcd.h
> @@ -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;
> diff --git a/drivers/usb/isp1760/isp1760-regs.h b/drivers/usb/isp1760/isp1760-regs.h
> index 3a6751197e970013..a5a442015887ce0b 100644
> --- a/drivers/usb/isp1760/isp1760-regs.h
> +++ b/drivers/usb/isp1760/isp1760-regs.h
> @@ -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
> diff --git a/drivers/usb/isp1760/isp1760-udc.h b/drivers/usb/isp1760/isp1760-udc.h
> index 22044e86bc0ecb84..609444bea306ba81 100644
> --- a/drivers/usb/isp1760/isp1760-udc.h
> +++ b/drivers/usb/isp1760/isp1760-udc.h
> @@ -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;
>
> --
> Alexey V. Vissarionov
> gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
> GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] usb: isp1760: increase buffer size to avoid overflow
2025-04-23 11:10 ` [PATCH v1] " Alexey V. Vissarionov
2025-04-24 9:15 ` Rui Miguel Silva
@ 2025-04-24 10:04 ` kernel test robot
1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2025-04-24 10:04 UTC (permalink / raw)
To: Alexey V. Vissarionov, Rui Miguel Silva
Cc: oe-kbuild-all, Alexey V. Vissarionov, Fedor Pchelkin,
Greg Kroah-Hartman, linux-usb, lvc-project
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/202504241715.9VNxWTQq-lkp@intel.com/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/202504241715.9VNxWTQq-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/202504241715.9VNxWTQq-lkp@intel.com/
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-24 10:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-22 23:00 [PATCH] usb: isp1760: increase buffer size to avoid overflow Alexey V. Vissarionov
2025-04-23 6:30 ` Fedor Pchelkin
2025-04-23 11:05 ` Alexey V. Vissarionov
2025-04-23 11:10 ` [PATCH v1] " Alexey V. Vissarionov
2025-04-24 9:15 ` Rui Miguel Silva
2025-04-24 10:04 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).