2022-09-09 04:44:14

by Li Zhong

[permalink] [raw]
Subject: [PATCH v1] drivers/usb/core/driver: check return value of usb_set_interface()

Check return value of usb_set_interface() and report error if it fails.
Otherwise usb_set_interface() may fail without any warnings.

This flaw was found using an experimental static analysis tool we are
developing. Report warnings when the function usb_set_interface() fails
can increase the dianosability.

Signed-off-by: Li Zhong <[email protected]>
---
drivers/usb/core/driver.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 7e7e119c253f..ee375b5eafe6 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1332,8 +1332,10 @@ static int usb_resume_interface(struct usb_device *udev,

/* Carry out a deferred switch to altsetting 0 */
if (intf->needs_altsetting0 && !intf->dev.power.is_prepared) {
- usb_set_interface(udev, intf->altsetting[0].
- desc.bInterfaceNumber, 0);
+ status = usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0);
+ if (status < 0)
+ dev_err(intf->dev, "usb_set_interface error %d\n",
+ status);
intf->needs_altsetting0 = 0;
}
goto done;
--
2.25.1


2022-09-09 05:31:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v1] drivers/usb/core/driver: check return value of usb_set_interface()

On Thu, Sep 08, 2022 at 09:42:14PM -0700, Li Zhong wrote:
> This flaw was found using an experimental static analysis tool we are
> developing. Report warnings when the function usb_set_interface() fails
> can increase the dianosability.

Please read Documentation/process/researcher-guidelines.rst for how to
properly submit such things. Until that is followed, we can not accept
stuff from you, sorry.

greg k-h

2022-09-09 10:30:00

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v1] drivers/usb/core/driver: check return value of usb_set_interface()

On Thu, Sep 08, 2022 at 09:42:14PM -0700, Li Zhong wrote:
> Check return value of usb_set_interface() and report error if it fails.
> Otherwise usb_set_interface() may fail without any warnings.
>
> This flaw was found using an experimental static analysis tool we are
> developing. Report warnings when the function usb_set_interface() fails
> can increase the dianosability.

How did you test this change?

2022-09-09 10:47:59

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1] drivers/usb/core/driver: check return value of usb_set_interface()

Hi Li,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on westeri-thunderbolt/next staging/staging-testing linus/master v6.0-rc4 next-20220908]
[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/Li-Zhong/drivers-usb-core-driver-check-return-value-of-usb_set_interface/20220909-124349
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-randconfig-a015
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/720d6f8c6938ee748288a012e3212b26962a7960
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Li-Zhong/drivers-usb-core-driver-check-return-value-of-usb_set_interface/20220909-124349
git checkout 720d6f8c6938ee748288a012e3212b26962a7960
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/usb/core/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/usb/core/driver.c:1337:13: error: passing 'struct device' to parameter of incompatible type 'const struct device *'; take the address with &
dev_err(intf->dev, "usb_set_interface error %d\n",
^~~~~~~~~
&
include/linux/dev_printk.h:144:44: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~
include/linux/dev_printk.h:110:11: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
^~~
include/linux/dev_printk.h:50:36: note: passing argument to parameter 'dev' here
void _dev_err(const struct device *dev, const char *fmt, ...);
^
1 error generated.


vim +1337 drivers/usb/core/driver.c

1316
1317 static int usb_resume_interface(struct usb_device *udev,
1318 struct usb_interface *intf, pm_message_t msg, int reset_resume)
1319 {
1320 struct usb_driver *driver;
1321 int status = 0;
1322
1323 if (udev->state == USB_STATE_NOTATTACHED)
1324 goto done;
1325
1326 /* Don't let autoresume interfere with unbinding */
1327 if (intf->condition == USB_INTERFACE_UNBINDING)
1328 goto done;
1329
1330 /* Can't resume it if it doesn't have a driver. */
1331 if (intf->condition == USB_INTERFACE_UNBOUND) {
1332
1333 /* Carry out a deferred switch to altsetting 0 */
1334 if (intf->needs_altsetting0 && !intf->dev.power.is_prepared) {
1335 status = usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0);
1336 if (status < 0)
> 1337 dev_err(intf->dev, "usb_set_interface error %d\n",
1338 status);
1339 intf->needs_altsetting0 = 0;
1340 }
1341 goto done;
1342 }
1343
1344 /* Don't resume if the interface is marked for rebinding */
1345 if (intf->needs_binding)
1346 goto done;
1347 driver = to_usb_driver(intf->dev.driver);
1348
1349 if (reset_resume) {
1350 if (driver->reset_resume) {
1351 status = driver->reset_resume(intf);
1352 if (status)
1353 dev_err(&intf->dev, "%s error %d\n",
1354 "reset_resume", status);
1355 } else {
1356 intf->needs_binding = 1;
1357 dev_dbg(&intf->dev, "no reset_resume for driver %s?\n",
1358 driver->name);
1359 }
1360 } else {
1361 status = driver->resume(intf);
1362 if (status)
1363 dev_err(&intf->dev, "resume error %d\n", status);
1364 }
1365
1366 done:
1367 dev_vdbg(&intf->dev, "%s: status %d\n", __func__, status);
1368
1369 /* Later we will unbind the driver and/or reprobe, if necessary */
1370 return status;
1371 }
1372

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (4.65 kB)
config (180.10 kB)
Download all attachments

2022-09-09 11:39:46

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1] drivers/usb/core/driver: check return value of usb_set_interface()

Hi Li,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on westeri-thunderbolt/next staging/staging-testing linus/master v6.0-rc4 next-20220909]
[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/Li-Zhong/drivers-usb-core-driver-check-return-value-of-usb_set_interface/20220909-124349
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-randconfig-a003 (https://download.01.org/0day-ci/archive/20220909/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/720d6f8c6938ee748288a012e3212b26962a7960
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Li-Zhong/drivers-usb-core-driver-check-return-value-of-usb_set_interface/20220909-124349
git checkout 720d6f8c6938ee748288a012e3212b26962a7960
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/usb/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from include/linux/device.h:15,
from drivers/usb/core/driver.c:28:
drivers/usb/core/driver.c: In function 'usb_resume_interface':
>> drivers/usb/core/driver.c:1337:45: error: incompatible type for argument 1 of '_dev_err'
1337 | dev_err(intf->dev, "usb_set_interface error %d\n",
| ~~~~^~~~~
| |
| struct device
include/linux/dev_printk.h:110:25: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
drivers/usb/core/driver.c:1337:33: note: in expansion of macro 'dev_err'
1337 | dev_err(intf->dev, "usb_set_interface error %d\n",
| ^~~~~~~
include/linux/dev_printk.h:50:36: note: expected 'const struct device *' but argument is of type 'struct device'
50 | void _dev_err(const struct device *dev, const char *fmt, ...);
| ~~~~~~~~~~~~~~~~~~~~~^~~


vim +/_dev_err +1337 drivers/usb/core/driver.c

1316
1317 static int usb_resume_interface(struct usb_device *udev,
1318 struct usb_interface *intf, pm_message_t msg, int reset_resume)
1319 {
1320 struct usb_driver *driver;
1321 int status = 0;
1322
1323 if (udev->state == USB_STATE_NOTATTACHED)
1324 goto done;
1325
1326 /* Don't let autoresume interfere with unbinding */
1327 if (intf->condition == USB_INTERFACE_UNBINDING)
1328 goto done;
1329
1330 /* Can't resume it if it doesn't have a driver. */
1331 if (intf->condition == USB_INTERFACE_UNBOUND) {
1332
1333 /* Carry out a deferred switch to altsetting 0 */
1334 if (intf->needs_altsetting0 && !intf->dev.power.is_prepared) {
1335 status = usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0);
1336 if (status < 0)
> 1337 dev_err(intf->dev, "usb_set_interface error %d\n",
1338 status);
1339 intf->needs_altsetting0 = 0;
1340 }
1341 goto done;
1342 }
1343
1344 /* Don't resume if the interface is marked for rebinding */
1345 if (intf->needs_binding)
1346 goto done;
1347 driver = to_usb_driver(intf->dev.driver);
1348
1349 if (reset_resume) {
1350 if (driver->reset_resume) {
1351 status = driver->reset_resume(intf);
1352 if (status)
1353 dev_err(&intf->dev, "%s error %d\n",
1354 "reset_resume", status);
1355 } else {
1356 intf->needs_binding = 1;
1357 dev_dbg(&intf->dev, "no reset_resume for driver %s?\n",
1358 driver->name);
1359 }
1360 } else {
1361 status = driver->resume(intf);
1362 if (status)
1363 dev_err(&intf->dev, "resume error %d\n", status);
1364 }
1365
1366 done:
1367 dev_vdbg(&intf->dev, "%s: status %d\n", __func__, status);
1368
1369 /* Later we will unbind the driver and/or reprobe, if necessary */
1370 return status;
1371 }
1372

--
0-DAY CI Kernel Test Service
https://01.org/lkp