Re: [PATCH] deprecate (un)register_ioctl32_conversion

[Michael S. Tsirkin]

Hello!
Quoting r. Christoph Hellwig (hch@xxxxxxxxxxxxx) "Re: [PATCH] deprecate (un)register_ioctl32_conversion":
> On Thu, Jan 06, 2005 at 05:22:48PM +0200, Michael S. Tsirkin wrote:
> > > +	if (!filp->f_op || !filp->f_op->ioctl)
> > > +		goto do_ioctl;
> > > +
> > > +	if (filp->f_op || filp->f_op->compat_ioctl) {
> > >  		error = filp->f_op->compat_ioctl(filp, cmd, arg);
> > >  		goto out_fput;
> > >  	}
> > 
> > So now if I dont have ->ioctl the ioctl_compat wont be called.
> > What if I only have unlocked_ioctl?
> 
> Indeed.  In my test setup I didn't have a driver using both.  So let's
> think a little more what checks we want.
> 
> The original intention (pre-patch) was that without an ioctl entry
> we'd skip the hash table lookup and skip right to trying the few standard
> ioctls.
> 
> So with ->compat_ioctl we should try that one first, then checking
> for either ->ioctl or ->unlocked_ioctl beeing there.  Like the patch
> below (this time it's actually untested because all my 64bit machines
> are in use):
> 
> 
> --- linux-2.6.10-mm2.orig/fs/compat.c	2005-01-06 11:40:18.831900000 +0100
> +++ linux-2.6.10-mm2/fs/compat.c	2005-01-06 16:36:17.340977664 +0100
> @@ -436,14 +436,15 @@
>  	if (!filp)
>  		goto out;
>  
> -	if (!filp->f_op) {
> -		if (!filp->f_op->ioctl)
> -			goto do_ioctl;
> -	} else if (filp->f_op->compat_ioctl) {
> +	if (filp->f_op && filp->f_op->compat_ioctl) {
>  		error = filp->f_op->compat_ioctl(filp, cmd, arg);
>  		goto out_fput;
>  	}
>  
> +	if (!filp->f_op ||
> +	    (!filp->f_op->ioctl && !filp->f_op->unlocked_ioctl))
> +		goto do_ioctl;
> +
>  	down_read(&ioctl32_sem);
>  	for (t = ioctl32_hash_table[ioctl32_hash(cmd)]; t; t = t->next) {
>  		if (t->cmd == cmd)
> 
> > 
> > 
> > MST
> ---end quoted text---


Amen to that. Since it conflicts with this change, here again is my patch
to make it possible for the compat_ioctl to drop back on the standard
conversions.
Applied on top of Christoph's last patch (above).

Signed-off-by: Michael S. Tsirkn <mst@xxxxxxxxxxxxxx>

diff -rup linux-2.6.10-mm2/fs/compat.c linux-2.6.10-ioctls/fs/compat.c
--- linux-2.6.10-mm2/fs/compat.c	2005-01-06 21:30:57.485167280 +0200
+++ linux-2.6.10-ioctls/fs/compat.c	2005-01-06 21:33:17.608865240 +0200
@@ -431,9 +431,10 @@ asmlinkage long compat_sys_ioctl(unsigne
 	if (!filp)
 		goto out;
 
 	if (filp->f_op && filp->f_op->compat_ioctl) {
 		error = filp->f_op->compat_ioctl(filp, cmd, arg);
-		goto out_fput;
+		if (error != -ENOIOCTLCMD)
+			goto out_fput;
 	}
 
 	if (!filp->f_op ||
diff -rup linux-2.6.10-mm2/fs/ioctl.c linux-2.6.10-ioctls/fs/ioctl.c
--- linux-2.6.10-mm2/fs/ioctl.c	2005-01-06 17:54:13.000000000 +0200
+++ linux-2.6.10-ioctls/fs/ioctl.c	2005-01-06 20:34:09.329285728 +0200
@@ -26,6 +26,9 @@ static long do_ioctl(struct file *filp, 
 
 	if (filp->f_op->unlocked_ioctl) {
 		error = filp->f_op->unlocked_ioctl(filp, cmd, arg);
+		if (error == -ENOIOCTLCMD)
+			error = -EINVAL;
+		goto out;
 	} else if (filp->f_op->ioctl) {
 		lock_kernel();
 		error = filp->f_op->ioctl(filp->f_dentry->d_inode,
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/