On 25.04.22 15:23, Bastien Nocera wrote:
> struct usb_memory {
> @@ -237,6 +238,9 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma)
> dma_addr_t dma_handle;
> int ret;
>
> + if (!connected(ps) || ps->revoked)
> + return -ENODEV;
> +
This lacks locking.
>
> +static int usbdev_revoke(struct usb_dev_state *ps)
> +{
> + struct usb_device *dev = ps->dev;
> + unsigned int ifnum;
> + struct async *as;
> +
> + if (ps->revoked)
> + return -ENODEV;
> + ps->revoked = true;
> +
> + usb_lock_device(dev);
And here you lock the device a second time. That is a bad idea.
> + for (ifnum = 0; ps->ifclaimed && ifnum < 8*sizeof(ps->ifclaimed);
> + ifnum++) {
> + if (test_bit(ifnum, &ps->ifclaimed))
> + releaseintf(ps, ifnum);
> + }
> + destroy_all_async(ps);
> + usb_unlock_device(dev);
> +
> + as = async_getcompleted(ps);
> + while (as) {
> + free_async(as);
> + as = async_getcompleted(ps);
> + }
> +
> + return 0;
> +}
Getting your file descriptor revoked should wake you up
from poll(), shouldn't it?
> +
> /*
> * NOTE: All requests here that have interface numbers as parameters
> * are assuming that somehow the configuration has been prevented from
> @@ -2619,7 +2660,7 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
> #endif
> }
>
> - if (!connected(ps)) {
> + if (!connected(ps) || ps->revoked) {
> usb_unlock_device(dev);
> return -ENODEV;
> }
> @@ -2779,6 +2820,11 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
> case USBDEVFS_WAIT_FOR_RESUME:
> ret = proc_wait_for_resume(ps);
> break;
> + case USBDEVFS_REVOKE:
You are still in usb_lock_device()
Regards
Oliver
