linux-kernel - Re: [GIT PULL v2] Preparation for BKL'ed ioctl removal

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100425180520.GD5375@nowhere>
Date:	Sun, 25 Apr 2010 20:05:22 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Arnd Bergmann <arnd@...db.de>, LKML <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Al Viro <viro@...iv.linux.org.uk>,
	Jan Blunck <jblunck@...e.de>, Ingo Molnar <mingo@...e.hu>,
	John Kacur <jkacur@...hat.com>
Subject: Re: [GIT PULL v2] Preparation for BKL'ed ioctl removal

On Sun, Apr 25, 2010 at 10:49:51AM -0700, Linus Torvalds wrote:
> 
> 
> On Sun, 25 Apr 2010, Frederic Weisbecker wrote:
> > 
> > And to prepare for that, are you ok with this scheme of:
> > 
> > -	.ioctl = foo,
> > +	.unlocked_ioctl = bkl_ioctl,
> > +	.bkl_ioctl	= foo,
> > 
> > ...done at the same time as the big rename patch.
> 
> Seriously, why not just
> 
> 	-	.ioctl = foo,
> 	+	.bkl_ioctl = foo
> 
> because that line of
> 
> 	+	.unlocked_ioctl = bkl_ioctl,
> 
> is just total and utter _garbage_.  There is zero reason for it.
> 
> In the long run (this is a year from now, when we rename "unlocked_ioctl" 
> back to just "ioctl"), the vfs_ioctl code will just do
> 
> 		struct file_operations *fops = filp->f_op;
> 
> 		if (!fops)
> 			return -ENOTTY;
> 
> 		if (fops->ioctl) {
> 			int error = fops->ioctl(...)
> 			if (error == -ENOIOCTLCMD)
> 				error = -EINVAL;
> 			return error;
> 		}
> 	#ifdef CONFIG_BKL
> 		if (fops->bkl_ioctl) {
> 			int error;
> 			lock_kernel();
> 			error = fops->bkl_ioctl(...)
> 			unlock_kernel();
> 			return error;
> 		}
> 	#endif
> 		return -ENOTTY;
> 
> and we're all done.
> 
> At NO point is there any advantage to that "bkl_ioctl" crap. It doesn't 
> help the legacy drivers (which won't even _compile_ unless CONFIG_BKL is 
> set anyway), it doesn't help the core code, it doesn't help _anybody_.
> 
> Not today, not tomorrow, not with CONFIG_BKL, and not without.
> 
> 			Linus


Well, we won't be able to get this bkl.ko but the desired effect of
having it was rather psychological than practical, as Arnd explained:
to make the dependency more visible and pull concerned people
into dropping the bkl from the drivers they are using.

But other than that, the final effect remains pretty the same so
it's not a big deal.

I'm ok with that.

Thanks for clarifying the situation!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/