[QUESTION] usb: usbfs SETINTERFACE during a runtime reset-resume

[QUESTION] usb: usbfs SETINTERFACE during a runtime reset-resume

[Cheng Yongkang]

Hi Alan,

I have a question about usbfs and runtime reset-resume. I am not sure
it is a bug, so I would like to ask before sending anything.

The kerneldoc of usb_reset_and_verify_device() says:

  Locking exception: This routine may also be called from within an
  autoresume handler.  Such usage won't conflict with other tasks
  holding the device lock because these tasks should always call
  usb_autopm_resume_device(), thereby preventing any unwanted
  autoresume.

(usb_autopm_resume_device() no longer exists; I assume
usb_autoresume_device() is meant.)

proc_setintf() runs under the device lock but takes no PM reference.
After USBDEVFS_ALLOW_SUSPEND, the device can be runtime suspended
while the file is open. If it then resumes through reset-resume,
finish_port_resume() sets USB_STATE_CONFIGURED before
usb_reset_and_verify_device() runs its restore loop. A SETINTERFACE
issued at that moment passes checkintf() and calls usb_set_interface()
while the restore loop is still reading intf->cur_altsetting and
calling usb_set_interface() to put the interfaces back into their old
altsettings.

On v7.3, with dummy_hcd and a gadget that forces reset-resume, I saw
1097 SETINTERFACE calls complete inside the resume window over ten
rounds, all with -EPROTO. There were no KASAN or lockdep reports, and
I did not see any corrupted state afterwards. I know 7794f486ed0b says
that I/O to a suspended device fails with an error code, so -EPROTO by
itself may be expected.

My questions are:

1. Is usbfs expected to follow the assumption in that comment, so
   that proc_setintf() should hold a PM reference while it changes the
   altsetting?

2. Or is it up to the program to call USBDEVFS_FORBID_SUSPEND or
   USBDEVFS_WAIT_FOR_RESUME first, so that this overlap with the
   restore loop is acceptable?

3. If it is the first, would taking usb_autoresume_device() around
   checkintf() and usb_set_interface() be the right fix? In my test
   the ioctl then waited until the resume finished and returned 0,
   which changes the current behavior of failing with an error.

Thanks,
Cheng Yongkang