Re: [RFC v1] USB: core: add USBDEVFS_REVOKE ioctl

[Oliver Neukum <oneukum@suse.com>]

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