Re: [PATCH] simplify networking fcntl

Re: [PATCH] simplify networking fcntl

[Andi Kleen]

I believe James Morris did this (clean up network fcntl) already in a more 
complex patchkit that also cleans up the SIGIO/SIGURG sending. 
Perhaps you coordinate with him.

-Andi

Re: [PATCH] simplify networking fcntl

[Matthew Wilcox]

On Sun, Jul 07, 2002 at 06:54:22PM +0200, Andi Kleen wrote:
> 
> I believe James Morris did this (clean up network fcntl) already in a more 
> complex patchkit that also cleans up the SIGIO/SIGURG sending. 
> Perhaps you coordinate with him.

Last I heard from James, he was having more trouble than he expected
making it work right.  an incremental improvement didn't seem
unreasonable.

-- 
Revolutions do not require corporate support.

Re: [PATCH] simplify networking fcntl

[James Morris]

On 7 Jul 2002, Andi Kleen wrote:

> 
> I believe James Morris did this (clean up network fcntl) already in a more 
> complex patchkit that also cleans up the SIGIO/SIGURG sending. 
> 

This is almost ready, I just need to complete a couple of higher priority 
tasks before incorporating some further suggestions and finalizing the 
patch (may be a week or so until I can get back to it).


- James
-- 
James Morris
<jmorris@intercode.com.au>

[PATCH] simplify networking fcntl

[Matthew Wilcox]

Sockets haven't had their own special ioctls since before linux 2.0.29.
sock_no_fcntl is only called for F_SETOWN, so it can stand some
simplification.

diff -urNX dontdiff linux-2.5.24/fs/fcntl.c linux-2.5.24-mm/fs/fcntl.c
--- linux-2.5.24/fs/fcntl.c	Sun Jun  9 06:09:49 2002
+++ linux-2.5.24-mm/fs/fcntl.c	Tue Jul  2 10:55:29 2002
@@ -347,10 +345,6 @@
 			err = fcntl_dirnotify(fd, filp, arg);
 			break;
 		default:
-			/* sockets need a few special fcntls. */
-			err = -EINVAL;
-			if (S_ISSOCK (filp->f_dentry->d_inode->i_mode))
-				err = sock_fcntl (filp, cmd, arg);
 			break;
 	}
 
diff -urNX dontdiff linux-2.5.24/net/core/sock.c linux-2.5.24-mm/net/core/sock.c
--- linux-2.5.24/net/core/sock.c	Sun Jun  2 18:44:52 2002
+++ linux-2.5.24-mm/net/core/sock.c	Tue Jul  2 10:37:58 2002
@@ -1048,32 +1048,17 @@
 	return -EOPNOTSUPP;
 }
 
-/* 
- * Note: if you add something that sleeps here then change sock_fcntl()
- *       to do proper fd locking.
- */
 int sock_no_fcntl(struct socket *sock, unsigned int cmd, unsigned long arg)
 {
-	struct sock *sk = sock->sk;
-
-	switch(cmd)
-	{
-		case F_SETOWN:
-			/*
-			 * This is a little restrictive, but it's the only
-			 * way to make sure that you can't send a sigurg to
-			 * another process.
-			 */
-			if (current->pgrp != -arg &&
-				current->pid != arg &&
-				!capable(CAP_KILL)) return(-EPERM);
-			sk->proc = arg;
-			return(0);
-		case F_GETOWN:
-			return(sk->proc);
-		default:
-			return(-EINVAL);
-	}
+	/*
+	 * TCP doesn't use the standard fasync method to deliver
+	 * SIGURG and SIGIO, so we check permissions at setup time.
+	 * This should be fixed.
+	 */
+	if (current->pgrp != -arg && current->pid != arg && !capable(CAP_KILL))
+		return -EPERM;
+	sock->sk->proc = arg;
+	return 0;
 }
 
 int sock_no_sendmsg(struct socket *sock, struct msghdr *m, int flags,

-- 
Revolutions do not require corporate support.

Re: [PATCH] simplify networking fcntl

[kuznet]

Hello!

> sock_no_fcntl is only called for F_SETOWN, so it can stand some
> simplification.

sk->proc. Sorry, generic F_SETOWN does not handle SIGURG.

Alexey

Re: [PATCH] simplify networking fcntl

[Matthew Wilcox]

On Sun, Jul 07, 2002 at 09:09:02PM +0400, kuznet@ms2.inr.ac.ru wrote:
> Hello!
> 
> > sock_no_fcntl is only called for F_SETOWN, so it can stand some
> > simplification.
> 
> sk->proc. Sorry, generic F_SETOWN does not handle SIGURG.

indeed -- did you read the patch?  i simplified sock_no_fcntl so it
_only_ handled F_SETOWN, which is the only time it's called.

-- 
Revolutions do not require corporate support.

Re: [PATCH] simplify networking fcntl

[kuznet]

Hello!

> indeed -- did you read the patch?  i simplified sock_no_fcntl so it
> _only_ handled F_SETOWN, which is the only time it's called.

Pardon, I interpreted it wrongly, decided you removed the call down,
which would be right thing to do, only taking care of SIGURG.
SIGIO is not a problem, this handled by generic code.

Andi reminded about patch by James Morris doing something with this.

Alexey