From linux-kernel-owner+willy=40w.ods.org-S261515AbVBADTo@vger.kernel.org Tue Feb 1 04:22:36 2005 Return-Path: Received: from vger.kernel.org (vger.kernel.org [12.107.209.244]) by mail.w.ods.org (8.12.9/8.12.1) with ESMTP id j113MWac027995 for ; Tue, 1 Feb 2005 04:22:34 +0100 (CET) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S261515AbVBADTo (ORCPT ); Mon, 31 Jan 2005 22:19:44 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S261516AbVBADTo (ORCPT ); Mon, 31 Jan 2005 22:19:44 -0500 Received: from mx1.redhat.com ([66.187.233.31]:21978 "EHLO mx1.redhat.com") by vger.kernel.org with ESMTP id S261515AbVBADS7 (ORCPT ); Mon, 31 Jan 2005 22:18:59 -0500 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id j113Ik8i024181; Mon, 31 Jan 2005 22:18:46 -0500 Received: from devserv.devel.redhat.com (devserv.devel.redhat.com [172.16.58.1]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id j113IkO11036; Mon, 31 Jan 2005 22:18:46 -0500 Received: from localhost.localdomain (vpn26-17.sfbay.redhat.com [172.16.26.17]) by devserv.devel.redhat.com (8.12.11/8.12.10) with SMTP id j113IiYr021562; Mon, 31 Jan 2005 22:18:44 -0500 Date: Mon, 31 Jan 2005 19:18:43 -0800 From: Pete Zaitcev To: Kaupo Arulo Cc: Sergey Vlasov , Meelis Roos , linux-usb-devel@lists.sourceforge.net, Stuart_Hayes@Dell.com, Meelis Roos , zaitcev@redhat.com, linux-kernel@vger.kernel.org Subject: Re: USB ioctl problem in 2.4.28 Message-ID: <20050131191843.118db7bd@localhost.localdomain> In-Reply-To: References: <20041129155155.53dfa072@lembas.zaitcev.lan> <20041130121836.GB2265@sirius.home> <20041130152041.GA2264@sirius.home> Organization: Red Hat, Inc. X-Mailer: Sylpheed-Claws 0.9.12cvs126.2 (GTK+ 2.4.14; i386-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org Status: RO Content-Length: 4766 Lines: 170 On Wed, 1 Dec 2004 14:57:41 +0200 (EET), Kaupo Arulo wrote: > We can use a macro, if it is really necessary No, I prefer it explicit. See the attached patch. I changed Sergey's patch just a little, so the invalid ioctls are detected outside the lock. > IMHO it is correct to return -ENOIOCTLCMD right away, not waiting for > freeing a mutex. Returning ENOIOCTLCMD is never correct, BTW, so I fixed that too. Someone please test this patch with modem_run and let me know if it works. I think it should be correct, but I would prefer a confirmation before this goes to Marcelo. Thank you in advance, -- Pete diff -urp -X dontdiff linux-2.4.29/drivers/usb/devio.c linux-2.4.29-usb/drivers/usb/devio.c --- linux-2.4.29/drivers/usb/devio.c 2004-11-22 23:04:18.000000000 -0800 +++ linux-2.4.29-usb/drivers/usb/devio.c 2005-01-31 17:57:45.342576912 -0800 @@ -1132,6 +1132,8 @@ static int proc_ioctl (struct dev_state /* ifno might usefully be passed ... */ retval = driver->ioctl (ps->dev, ctrl.ioctl_code, buf); /* size = min_t(int, size, retval)? */ + if (retval == -ENOIOCTLCMD) + retval = -ENOTTY; } } @@ -1146,24 +1148,10 @@ static int proc_ioctl (struct dev_state return retval; } -static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg) +static int usbdev_ioctl_exclusive(struct dev_state *ps, struct inode *inode, + unsigned int cmd, unsigned long arg) { - struct dev_state *ps = (struct dev_state *)file->private_data; - int ret = -ENOIOCTLCMD; - - if (!(file->f_mode & FMODE_WRITE)) - return -EPERM; - down_read(&ps->devsem); - if (!ps->dev) { - up_read(&ps->devsem); - return -ENODEV; - } - - /* - * grab device's exclusive_access mutex to prevent its driver from - * using this device while it is being accessed by us. - */ - down(&ps->dev->exclusive_access); + int ret; switch (cmd) { case USBDEVFS_CONTROL: @@ -1194,14 +1182,6 @@ static int usbdev_ioctl(struct inode *in inode->i_mtime = CURRENT_TIME; break; - case USBDEVFS_GETDRIVER: - ret = proc_getdriver(ps, (void *)arg); - break; - - case USBDEVFS_CONNECTINFO: - ret = proc_connectinfo(ps, (void *)arg); - break; - case USBDEVFS_SETINTERFACE: ret = proc_setintf(ps, (void *)arg); break; @@ -1220,6 +1200,53 @@ static int usbdev_ioctl(struct inode *in ret = proc_unlinkurb(ps, (void *)arg); break; + case USBDEVFS_CLAIMINTERFACE: + ret = proc_claiminterface(ps, (void *)arg); + break; + + case USBDEVFS_RELEASEINTERFACE: + ret = proc_releaseinterface(ps, (void *)arg); + break; + + case USBDEVFS_IOCTL: + ret = proc_ioctl(ps, (void *) arg); + break; + + default: + ret = -ENOTTY; + } + return ret; +} + +static int usbdev_ioctl(struct inode *inode, struct file *file, + unsigned int cmd, unsigned long arg) +{ + struct dev_state *ps = file->private_data; + int ret; + + if (!(file->f_mode & FMODE_WRITE)) + return -EPERM; + down_read(&ps->devsem); + if (!ps->dev) { + up_read(&ps->devsem); + return -ENODEV; + } + + /* + * Some ioctls don't touch the device and can be called without + * grabbing its exclusive_access mutex; they are handled in this + * switch. Other ioctls which need exclusive_access are handled in + * usbdev_ioctl_exclusive(). + */ + switch (cmd) { + case USBDEVFS_GETDRIVER: + ret = proc_getdriver(ps, (void *)arg); + break; + + case USBDEVFS_CONNECTINFO: + ret = proc_connectinfo(ps, (void *)arg); + break; + case USBDEVFS_REAPURB: ret = proc_reapurb(ps, (void *)arg); break; @@ -1232,19 +1259,28 @@ static int usbdev_ioctl(struct inode *in ret = proc_disconnectsignal(ps, (void *)arg); break; + case USBDEVFS_CONTROL: + case USBDEVFS_BULK: + case USBDEVFS_RESETEP: + case USBDEVFS_RESET: + case USBDEVFS_CLEAR_HALT: + case USBDEVFS_SETINTERFACE: + case USBDEVFS_SETCONFIGURATION: + case USBDEVFS_SUBMITURB: + case USBDEVFS_DISCARDURB: case USBDEVFS_CLAIMINTERFACE: - ret = proc_claiminterface(ps, (void *)arg); - break; - case USBDEVFS_RELEASEINTERFACE: - ret = proc_releaseinterface(ps, (void *)arg); - break; - case USBDEVFS_IOCTL: - ret = proc_ioctl(ps, (void *) arg); + ret = -ERESTARTSYS; + if (down_interruptible(&ps->dev->exclusive_access) == 0) { + ret = usbdev_ioctl_exclusive(ps, inode, cmd, arg); + up(&ps->dev->exclusive_access); + } break; + + default: + ret = -ENOTTY; } - up(&ps->dev->exclusive_access); up_read(&ps->devsem); if (ret >= 0) inode->i_atime = CURRENT_TIME; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/