netdev
[Top] [All Lists]

Re: [PATCH] minor socket ioctl cleanup for 2.5.30

To: James Morris <jmorris@xxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] minor socket ioctl cleanup for 2.5.30
From: Matthew Wilcox <willy@xxxxxxxxxx>
Date: Thu, 8 Aug 2002 18:42:06 +0100
Cc: Matthew Wilcox <willy@xxxxxxxxxx>, kuznet@xxxxxxxxxxxxx, davem@xxxxxxxxxx, netdev@xxxxxxxxxxx
In-reply-to: <Mutt.LNX.4.44.0208090320010.20027-100000@xxxxxxxxxxxxxxxxxxxxxxxxxx>; from jmorris@xxxxxxxxxxxxxxxx on Fri, Aug 09, 2002 at 03:26:10AM +1000
References: <20020808181717.P24631@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <Mutt.LNX.4.44.0208090320010.20027-100000@xxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: owner-netdev@xxxxxxxxxxx
User-agent: Mutt/1.2.5.1i
On Fri, Aug 09, 2002 at 03:26:10AM +1000, James Morris wrote:
> Yes, this patch is complete (see below), but is waiting on another fix to
> go in (which I sent you recently).  The unification of the ioctl code also
> now means SIOCSPGRP works exactly the same as F_SETOWN (i.e. it also works
> now for sigio).

this patch is for information purposes only, right?  ;-)

> diff -urN -X dontdiff linux-2.5.27.orig/fs/fcntl.c linux-2.5.27.w1/fs/fcntl.c
> --- linux-2.5.27.orig/fs/fcntl.c      Sun Jul 21 10:57:51 2002
> +++ linux-2.5.27.w1/fs/fcntl.c        Sun Jul 28 18:23:03 2002
> @@ -259,6 +259,18 @@
>       return 0;
>  }
>  
> +long f_getown(struct file *filp)
> +{
> +     return filp->f_owner.pid;
> +}

i still think this function shouldn't exist -- a direct reference is
no problem.

>               case F_SETOWN:
> -                     lock_kernel();
> -                     filp->f_owner.pid = arg;
> -                     filp->f_owner.uid = current->uid;
> -                     filp->f_owner.euid = current->euid;
>                       err = 0;
> -                     if (S_ISSOCK (filp->f_dentry->d_inode->i_mode))
> -                             err = sock_fcntl (filp, F_SETOWN, arg);
> +                     lock_kernel();
> +                     f_setown(filp, arg);
>                       unlock_kernel();

this is now hooked by the LSM folks so patch will need to be updated for
2.5.30.

> diff -urN -X dontdiff linux-2.5.27.orig/net/socket.c 
> linux-2.5.27.w1/net/socket.c
> --- linux-2.5.27.orig/net/socket.c    Sat Jul  6 11:01:02 2002
> +++ linux-2.5.27.w1/net/socket.c      Sun Jul 28 22:08:37 2002
> +     switch(cmd) {
> +     case FIOSETOWN:
> +     case SIOCSPGRP:
> +             if (get_user(pid, (int *)arg))
> +                     err = -EFAULT;
> +             else
> +                     f_setown(sock->file, pid);
> +             break;

i'm pretty sure you need a lock_kernel + unlock_kernel around the else.
if two people are doing a F_SETOWN / SIOCSPGRP at the same time, you could
have a race.

-- 
Revolutions do not require corporate support.


<Prev in Thread] Current Thread [Next in Thread>