回复: [PATCH] USB: usblp: Add device status detection in usblp_poll()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>________________________________________
>发件人: Pete Zaitcev <zaitcev@xxxxxxxxxx>
>发送时间: 2021年3月3日 13:12
>收件人: Zhang, Qiang
>抄送: Greg KH; linux-usb@xxxxxxxxxxxxxxx; zaitcev@xxxxxxxxxx
>主题: Re: [PATCH] USB: usblp: Add device status detection in >usblp_poll()
>
>[Please note: This e-mail is from an EXTERNAL e-mail address]
>
>On Tue, 2 Mar 2021 07:41:07 +0000
>"Zhang, Qiang" <Qiang.Zhang@xxxxxxxxxxxxx> wrote:

> and also I find  similar usage in usb/class/usbtmc.c

>Seems like a bug indeed, but I don't like the example in usbtmc.c.
>Please let me know if the following is acceptable:
 
I agree with this change .

Thanks
Qiang

>
>commit 83591ac63bc666a44f250b43af6c0f5a1e001841
>Author: Pete Zaitcev <zaitcev@xxxxxxxxxxxxxxxxx>
>Date:   Tue Mar 2 23:00:28 2021 -0600
>
>    usblp: fix a hang in poll() if disconnected
>
>   Apparently an application that opens a device and calls select()
>    on it, will hang if the decice is disconnected. It's a little
>    surprising that we had this bug for 15 years, but apparently
>    nobody ever uses select() with a printer: only write() and read(),
>    and those work fine. Well, you can also select() with a timeout.
>
>    The fix is modeled after devio.c. A few drivers check the
>    condition first, then do not add the wait queue in case the
>    device is disconnected. We doubt that's completely race-free.
>    So, this patch adds the process first, then locks properly
>    and checks for the disconnect.
>
>    Reported-by: Zqiang <qiang.zhang@xxxxxxxxxxxxx>
>    Signed-off-by: Pete Zaitcev <zaitcev@xxxxxxxxxx>
>
>diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
>index fd87405adbed..5733a0067f5b 100644
>--- a/drivers/usb/class/usblp.c
>+++ b/drivers/usb/class/usblp.c
>@@ -494,16 +494,24 @@ static int usblp_release(struct inode *inode, >struct file *file)
>/* No kernel lock - fine */
> static __poll_t usblp_poll(struct file *file, struct poll_table_struct >*wait)
> {
>-       __poll_t ret;
>+       struct usblp *usblp = file->private_data;
>+       __poll_t ret = 0;
>        unsigned long flags;
>
>-       struct usblp *usblp = file->private_data;
>        /* Should we check file->f_mode & FMODE_WRITE before >poll_wait()? */
>        poll_wait(file, &usblp->rwait, wait);
>        poll_wait(file, &usblp->wwait, wait);
>+
>+       mutex_lock(&usblp->mut);
>+       if (!usblp->present)
>+               ret != EPOLLHUP;
>+       mutex_unlock(&usblp->mut);
>+
>        spin_lock_irqsave(&usblp->lock, flags);
>-       ret = ((usblp->bidir && usblp->rcomplete) ? EPOLLIN  | >EPOLLRDNORM : 0) |
>-          ((usblp->no_paper || usblp->wcomplete) ? EPOLLOUT | >EPOLLWRNORM : 0);
>+       if (usblp->bidir && usblp->rcomplete)
>+               ret |= EPOLLIN  | EPOLLRDNORM;
>+       if (usblp->no_paper || usblp->wcomplete)
>+               ret |= EPOLLOUT | EPOLLWRNORM;
>        spin_unlock_irqrestore(&usblp->lock, flags);
>        return ret;
> }





[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux