2023-07-01 15:02:32

by Zhang Shurong

[permalink] [raw]
Subject: [PATCH] usb: r8a66597-hcd: host: fix port index underflow and UBSAN complains

If wIndex is 0 (and it often is), these calculations underflow and
UBSAN complains, here resolve this by not decrementing the index when
it is equal to 0.

Similar commit 85e3990bea49 ("USB: EHCI: avoid undefined pointer
arithmetic and placate UBSAN")

Signed-off-by: Zhang Shurong <[email protected]>
---
drivers/usb/host/r8a66597-hcd.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
index 9f4bf8c5f8a5..a65e0d995a4b 100644
--- a/drivers/usb/host/r8a66597-hcd.c
+++ b/drivers/usb/host/r8a66597-hcd.c
@@ -2141,9 +2141,11 @@ static int r8a66597_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
{
struct r8a66597 *r8a66597 = hcd_to_r8a66597(hcd);
int ret;
- int port = (wIndex & 0x00FF) - 1;
- struct r8a66597_root_hub *rh = &r8a66597->root_hub[port];
unsigned long flags;
+ u32 port_index = wIndex & 0xFF;
+
+ int port -= (port_index > 0);
+ struct r8a66597_root_hub *rh = &r8a66597->root_hub[port];

ret = 0;

--
2.41.0



2023-07-01 16:12:27

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] usb: r8a66597-hcd: host: fix port index underflow and UBSAN complains

Hi Zhang,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus westeri-thunderbolt/next linus/master v6.4 next-20230630]
[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/Zhang-Shurong/usb-r8a66597-hcd-host-fix-port-index-underflow-and-UBSAN-complains/20230701-223726
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/tencent_71A3B792C0AA3D9E148E517B24BC6E006A09%40qq.com
patch subject: [PATCH] usb: r8a66597-hcd: host: fix port index underflow and UBSAN complains
config: riscv-randconfig-r035-20230701 (https://download.01.org/0day-ci/archive/20230701/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230701/[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 error/warnings (new ones prefixed by >>):

drivers/usb/host/r8a66597-hcd.c: In function 'r8a66597_hub_control':
>> drivers/usb/host/r8a66597-hcd.c:2147:18: error: expected '=', ',', ';', 'asm' or '__attribute__' before '-=' token
2147 | int port -= (port_index > 0);
| ^~
>> drivers/usb/host/r8a66597-hcd.c:2147:18: error: expected expression before '-=' token
>> drivers/usb/host/r8a66597-hcd.c:2148:60: error: 'port' undeclared (first use in this function)
2148 | struct r8a66597_root_hub *rh = &r8a66597->root_hub[port];
| ^~~~
drivers/usb/host/r8a66597-hcd.c:2148:60: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/usb/host/r8a66597-hcd.c:2148:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
2148 | struct r8a66597_root_hub *rh = &r8a66597->root_hub[port];
| ^~~~~~


vim +2147 drivers/usb/host/r8a66597-hcd.c

2138
2139 static int r8a66597_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
2140 u16 wIndex, char *buf, u16 wLength)
2141 {
2142 struct r8a66597 *r8a66597 = hcd_to_r8a66597(hcd);
2143 int ret;
2144 unsigned long flags;
2145 u32 port_index = wIndex & 0xFF;
2146
> 2147 int port -= (port_index > 0);
> 2148 struct r8a66597_root_hub *rh = &r8a66597->root_hub[port];
2149
2150 ret = 0;
2151
2152 spin_lock_irqsave(&r8a66597->lock, flags);
2153 switch (typeReq) {
2154 case ClearHubFeature:
2155 case SetHubFeature:
2156 switch (wValue) {
2157 case C_HUB_OVER_CURRENT:
2158 case C_HUB_LOCAL_POWER:
2159 break;
2160 default:
2161 goto error;
2162 }
2163 break;
2164 case ClearPortFeature:
2165 if (wIndex > r8a66597->max_root_hub)
2166 goto error;
2167 if (wLength != 0)
2168 goto error;
2169
2170 switch (wValue) {
2171 case USB_PORT_FEAT_ENABLE:
2172 rh->port &= ~USB_PORT_STAT_POWER;
2173 break;
2174 case USB_PORT_FEAT_SUSPEND:
2175 break;
2176 case USB_PORT_FEAT_POWER:
2177 r8a66597_port_power(r8a66597, port, 0);
2178 break;
2179 case USB_PORT_FEAT_C_ENABLE:
2180 case USB_PORT_FEAT_C_SUSPEND:
2181 case USB_PORT_FEAT_C_CONNECTION:
2182 case USB_PORT_FEAT_C_OVER_CURRENT:
2183 case USB_PORT_FEAT_C_RESET:
2184 break;
2185 default:
2186 goto error;
2187 }
2188 rh->port &= ~(1 << wValue);
2189 break;
2190 case GetHubDescriptor:
2191 r8a66597_hub_descriptor(r8a66597,
2192 (struct usb_hub_descriptor *)buf);
2193 break;
2194 case GetHubStatus:
2195 *buf = 0x00;
2196 break;
2197 case GetPortStatus:
2198 if (wIndex > r8a66597->max_root_hub)
2199 goto error;
2200 *(__le32 *)buf = cpu_to_le32(rh->port);
2201 break;
2202 case SetPortFeature:
2203 if (wIndex > r8a66597->max_root_hub)
2204 goto error;
2205 if (wLength != 0)
2206 goto error;
2207
2208 switch (wValue) {
2209 case USB_PORT_FEAT_SUSPEND:
2210 break;
2211 case USB_PORT_FEAT_POWER:
2212 r8a66597_port_power(r8a66597, port, 1);
2213 rh->port |= USB_PORT_STAT_POWER;
2214 break;
2215 case USB_PORT_FEAT_RESET: {
2216 struct r8a66597_device *dev = rh->dev;
2217
2218 rh->port |= USB_PORT_STAT_RESET;
2219
2220 disable_r8a66597_pipe_all(r8a66597, dev);
2221 free_usb_address(r8a66597, dev, 1);
2222
2223 r8a66597_mdfy(r8a66597, USBRST, USBRST | UACT,
2224 get_dvstctr_reg(port));
2225 mod_timer(&r8a66597->rh_timer,
2226 jiffies + msecs_to_jiffies(50));
2227 }
2228 break;
2229 default:
2230 goto error;
2231 }
2232 rh->port |= 1 << wValue;
2233 break;
2234 default:
2235 error:
2236 ret = -EPIPE;
2237 break;
2238 }
2239
2240 spin_unlock_irqrestore(&r8a66597->lock, flags);
2241 return ret;
2242 }
2243

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

2023-07-01 17:17:35

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] usb: r8a66597-hcd: host: fix port index underflow and UBSAN complains

Hi Zhang,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus westeri-thunderbolt/next linus/master v6.4 next-20230630]
[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/Zhang-Shurong/usb-r8a66597-hcd-host-fix-port-index-underflow-and-UBSAN-complains/20230701-223726
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/tencent_71A3B792C0AA3D9E148E517B24BC6E006A09%40qq.com
patch subject: [PATCH] usb: r8a66597-hcd: host: fix port index underflow and UBSAN complains
config: mips-randconfig-r003-20230701 (https://download.01.org/0day-ci/archive/20230702/[email protected]/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230702/[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 errors (new ones prefixed by >>):

>> drivers/usb/host/r8a66597-hcd.c:2147:11: error: invalid '-=' at end of declaration; did you mean '='?
int port -= (port_index > 0);
^~
=
1 error generated.


vim +2147 drivers/usb/host/r8a66597-hcd.c

2138
2139 static int r8a66597_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
2140 u16 wIndex, char *buf, u16 wLength)
2141 {
2142 struct r8a66597 *r8a66597 = hcd_to_r8a66597(hcd);
2143 int ret;
2144 unsigned long flags;
2145 u32 port_index = wIndex & 0xFF;
2146
> 2147 int port -= (port_index > 0);
2148 struct r8a66597_root_hub *rh = &r8a66597->root_hub[port];
2149
2150 ret = 0;
2151
2152 spin_lock_irqsave(&r8a66597->lock, flags);
2153 switch (typeReq) {
2154 case ClearHubFeature:
2155 case SetHubFeature:
2156 switch (wValue) {
2157 case C_HUB_OVER_CURRENT:
2158 case C_HUB_LOCAL_POWER:
2159 break;
2160 default:
2161 goto error;
2162 }
2163 break;
2164 case ClearPortFeature:
2165 if (wIndex > r8a66597->max_root_hub)
2166 goto error;
2167 if (wLength != 0)
2168 goto error;
2169
2170 switch (wValue) {
2171 case USB_PORT_FEAT_ENABLE:
2172 rh->port &= ~USB_PORT_STAT_POWER;
2173 break;
2174 case USB_PORT_FEAT_SUSPEND:
2175 break;
2176 case USB_PORT_FEAT_POWER:
2177 r8a66597_port_power(r8a66597, port, 0);
2178 break;
2179 case USB_PORT_FEAT_C_ENABLE:
2180 case USB_PORT_FEAT_C_SUSPEND:
2181 case USB_PORT_FEAT_C_CONNECTION:
2182 case USB_PORT_FEAT_C_OVER_CURRENT:
2183 case USB_PORT_FEAT_C_RESET:
2184 break;
2185 default:
2186 goto error;
2187 }
2188 rh->port &= ~(1 << wValue);
2189 break;
2190 case GetHubDescriptor:
2191 r8a66597_hub_descriptor(r8a66597,
2192 (struct usb_hub_descriptor *)buf);
2193 break;
2194 case GetHubStatus:
2195 *buf = 0x00;
2196 break;
2197 case GetPortStatus:
2198 if (wIndex > r8a66597->max_root_hub)
2199 goto error;
2200 *(__le32 *)buf = cpu_to_le32(rh->port);
2201 break;
2202 case SetPortFeature:
2203 if (wIndex > r8a66597->max_root_hub)
2204 goto error;
2205 if (wLength != 0)
2206 goto error;
2207
2208 switch (wValue) {
2209 case USB_PORT_FEAT_SUSPEND:
2210 break;
2211 case USB_PORT_FEAT_POWER:
2212 r8a66597_port_power(r8a66597, port, 1);
2213 rh->port |= USB_PORT_STAT_POWER;
2214 break;
2215 case USB_PORT_FEAT_RESET: {
2216 struct r8a66597_device *dev = rh->dev;
2217
2218 rh->port |= USB_PORT_STAT_RESET;
2219
2220 disable_r8a66597_pipe_all(r8a66597, dev);
2221 free_usb_address(r8a66597, dev, 1);
2222
2223 r8a66597_mdfy(r8a66597, USBRST, USBRST | UACT,
2224 get_dvstctr_reg(port));
2225 mod_timer(&r8a66597->rh_timer,
2226 jiffies + msecs_to_jiffies(50));
2227 }
2228 break;
2229 default:
2230 goto error;
2231 }
2232 rh->port |= 1 << wValue;
2233 break;
2234 default:
2235 error:
2236 ret = -EPIPE;
2237 break;
2238 }
2239
2240 spin_unlock_irqrestore(&r8a66597->lock, flags);
2241 return ret;
2242 }
2243

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