Messages in this thread |  | | | From | Arnd Bergmann <> | | Subject | Re: [PATCH 6/6] procfs: Kill the bkl in ioctl | | Date | Tue, 30 Mar 2010 11:33:40 +0100 |
| |
On Tuesday 30 March 2010, Frederic Weisbecker wrote: > On Tue, Mar 30, 2010 at 09:38:11AM +0300, Alexey Dobriyan wrote: > > On Tue, Mar 30, 2010 at 9:20 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote: > > > --- a/fs/proc/inode.c > > > +++ b/fs/proc/inode.c > > > @@ -231,9 +231,9 @@ static long proc_reg_unlocked_ioctl(struct file *file, unsigned int cmd, unsigne > > > if (rv == -ENOIOCTLCMD) > > > rv = -EINVAL; > > > } else if (ioctl) { > > > - lock_kernel(); > > > + WARN_ONCE(1, "Procfs ioctl handlers must use unlocked_ioctl, " > > > + "%pf will be called without the Bkl held\n", ioctl); > > > rv = ioctl(file->f_path.dentry->d_inode, file, cmd, arg); > > > - unlock_kernel(); > > > > Then delete the branch. > > Or go through formal feature-removal procedure. > > > I thought about it. I even started to write something in this > feature-removal file but realized that I can't remove the > .ioctl() callback from file operations. We still need to check > the user hasn't made the mistake of implementing it. > > What I can plan as a feature removal, though, is to keep the warning > but don't actually call the ioctl.
I believe we can actually remove ioctl from file_operations. The patch I did to convert all users to ".unlocked_ioctl = default_ioctl," should really catch all cases, and I think we can enforce this by renaming fops->ioctl to locked_ioctl or old_ioctl to make sure we didn't miss any, and then mandate that this one is only used when unlocked_ioctl is set to default_ioctl.
I also remember going through procfs ioctl operations some time ago and finding exactly three users, which I believe are the same ones that Frederic found.
Arnd
|  |