xfs
[Top] [All Lists]

Re: bug 280 / your patch

To: Eric Sandeen <sandeen@xxxxxxx>
Subject: Re: bug 280 / your patch
From: Gustavo Homem <gustavo@xxxxxxxxxxxx>
Date: Sun, 14 Nov 2004 19:12:06 +0000
Cc: linux-xfs@xxxxxxxxxxx, s.hetze@xxxxxxxxxxx, hch@xxxxxxx, core@xxxxxxxxxxxx
In-reply-to: <41978E82.5030902@xxxxxxx>
References: <200411141601.01782.gustavo@xxxxxxxxxxxx> <41978E82.5030902@xxxxxxx>
Sender: linux-xfs-bounce@xxxxxxxxxxx
User-agent: KMail/1.5.3
Hello,

Thanks for your answer.

On Sunday 14 November 2004 16:57, Eric Sandeen wrote:
> Gustavo Homem wrote:
> > Hello,
> >
> > A message by Eric Sandeen,
> >
> > http://oss.sgi.com/archives/linux-xfs/2002-04/msg00234.html
> >
> > on
> >
> > Fri, 19 Apr 2002 14:49:53 -0500
> >
> > suggests that
> >
> > http://oss.sgi.com/bugzilla/show_bug.cgi?id=280
> >
> > was fixed.
>
> Well, I think that one is slightly different.

You are right, endeed. The bug is fixed if the directory doesn't contain any 
ACLs...

I tried changing the values of /proc/sys/fs/xfs/irix_sgid_inherit and it works 
as expected if there are no ACLs on the main directory.

Now that you sent your original patch I found that the related code on 
xfs_inode.c () is equal both on current CVS (Rev 1.406) and on the linux 
2.4.22-mdk source:

        if (XFS_INHERIT_GID(pip, vp->v_vfsp)) {
                ip->i_d.di_gid = pip->i_d.di_gid;
                if ((pip->i_d.di_mode & S_ISGID) && (mode & S_IFMT) == S_IFDIR) 
{ 
                        ip->i_d.di_mode |= S_ISGID;
                }
        }

// what may be wrong here ? Might something be wrong with "mode" (because of 
// the ACLs) so that (mode & S_IFMT) != S_FDIR ?

// it is important to note that if the user creating the file belongs to the
// directory group s-inheritance will work regardless of ACLs being present 
// or not. How is mode affected by this ?

// ******** doesn't matter if irix_sgid_inherit equals 0 ********

        /*
         * If the group ID of the new file does not match the effective group
         * ID or one of the supplementary group IDs, the S_ISGID bit is cleared
         * (and only if the irix_sgid_inherit compatibility variable is set).
         */
        if ((irix_sgid_inherit) &&
            (ip->i_d.di_mode & S_ISGID) &&
            (!in_group_p((gid_t)ip->i_d.di_gid))) {
                ip->i_d.di_mode &= ~S_ISGID;
        }

// ******** / doesn't matter if irix_sgid_inherit equals 0 ********

Thank you very much
Gustavo


>
> However this bug was opened a long time after, on
>
> > 2003-09-15 08:35 PDT
> >
> > and is still marked as OPEN. I can reproduce this bug with kernel
> > 2.4.22-21mdk.
>
> Just to be sure, if you could check it in the latest cvs kernel, or even
> the latest kernel.org kernel, that would be useful information.
>
> > I took a look at the webCVS interface but I can't request version 1.335
> > of xfs_inode.c
>
> Here is the patch as it was checked in in 2002.  However, I don't think
> it addresses the aclc problem:
>
> ===========================================================================
> linux/Documentation/filesystems/xfs.txt
> ===========================================================================
>
> --- a/linux/Documentation/filesystems/xfs.txt   2004-11-14
> 10:49:21.000000000 -0600
> +++ b/linux/Documentation/filesystems/xfs.txt   2004-11-14
> 10:49:21.000000000 -0600
> @@ -32,6 +32,12 @@
>     dmapi/xdsm
>          Enable the DMAPI (Data Management API) event callouts.
>
> +  irixsgid
> +       Do not inherit the ISGID bit on subdirectories of ISGID
> +       directories, if the process creating the subdirectory
> +       is not a member of the parent directory group ID.
> +       This matches IRIX behavior.
> +
>     logbufs=value
>          Set the number of in-memory log buffers.  Valid numbers range
>          from 2-8 inclusive.
>
> ===========================================================================
> linux/fs/xfs/linux/xfs_super.c
> ===========================================================================
>
> --- a/linux/fs/xfs/linux/xfs_super.c    2004-11-14 10:49:21.000000000 -0600
> +++ b/linux/fs/xfs/linux/xfs_super.c    2004-11-14 10:49:21.000000000 -0600
> @@ -113,6 +113,7 @@
>   #define MNTOPT_RO       "ro"            /* read only */
>   #define MNTOPT_RW       "rw"            /* read/write */
>   #define MNTOPT_NOUUID   "nouuid"       /* Ignore FS uuid */
> +#define MNTOPT_IRIXSGID "irixsgid"     /* Irix-style sgid inheritance */
>
>   STATIC int
>   xfs_parseargs(
> @@ -239,6 +240,8 @@
>                          args->flags |= MS_NOSUID;
>                  } else if (!strcmp(this_char, MNTOPT_NOUUID)) {
>                          args->flags |= XFSMNT_NOUUID;
> +               } else if (!strcmp(this_char, MNTOPT_IRIXSGID)) {
> +                       args->flags |= XFSMNT_IRIXSGID;
>                  } else {
>                          printk("XFS: unknown mount option [%s].\n",
> this_char);
>                          return rval;
>
> ===========================================================================
> linux/fs/xfs/xfs_clnt.h
> ===========================================================================
>
> --- a/linux/fs/xfs/xfs_clnt.h   2004-11-14 10:49:21.000000000 -0600
> +++ b/linux/fs/xfs/xfs_clnt.h   2004-11-14 10:49:21.000000000 -0600
> @@ -123,6 +123,7 @@
>   #define XFSMNT_NOUUID          0x01000000      /* Ignore fs uuid */
>   #define XFSMNT_32BITINODES     0x02000000      /* restrict inodes to 32
>                                                   * bits of address space
> */ +#define XFSMNT_IRIXSGID                0x04000000      /* Irix-style
> sgid inheritance */
>
>   /* Did we get any args for CXFS to consume? */
>   #define XFSARGS_FOR_CXFSARR(ap)                \
>
> ===========================================================================
> linux/fs/xfs/xfs_inode.c
> ===========================================================================
>
> --- a/linux/fs/xfs/xfs_inode.c  2004-11-14 10:49:21.000000000 -0600
> +++ b/linux/fs/xfs/xfs_inode.c  2004-11-14 10:49:21.000000000 -0600
> @@ -1098,10 +1098,11 @@
>          /*
>           * If the group ID of the new file does not match the effective
> group
>           * ID or one of the supplementary group IDs, the ISGID bit is
> -        * cleared.
> +        * cleared if the "irixsgid" mount option is set.
>           */
>          if (ip->i_d.di_mode & ISGID) {
> -               if (!in_group_p((gid_t)ip->i_d.di_gid) &&
> !capable(CAP_FSETID)){
> +               if (!in_group_p((gid_t)ip->i_d.di_gid)
> +                   && (ip->i_mount->m_flags & XFS_MOUNT_IRIXSGID)) {
>                          ip->i_d.di_mode &= ~ISGID;
>                  }
>          }
>
> ===========================================================================
> linux/fs/xfs/xfs_mount.h
> ===========================================================================
>
> --- a/linux/fs/xfs/xfs_mount.h  2004-11-14 10:49:21.000000000 -0600
> +++ b/linux/fs/xfs/xfs_mount.h  2004-11-14 10:49:21.000000000 -0600
> @@ -341,6 +341,7 @@
>   #define XFS_MOUNT_NOUUID       0x00004000      /* ignore uuid during
> mount */
>   #define XFS_MOUNT_32BITINODES  0x00008000      /* do not create inodes
> above
>                                                   * 32 bits in size */
> +#define XFS_MOUNT_IRIXSGID     0x00010000      /* Irix-style sgid
> inheritance */
>
>   /*
>    * Flags for m_cxfstype
>
> ===========================================================================
> linux/fs/xfs/xfs_vfsops.c
> ===========================================================================
>
> --- a/linux/fs/xfs/xfs_vfsops.c 2004-11-14 10:49:21.000000000 -0600
> +++ b/linux/fs/xfs/xfs_vfsops.c 2004-11-14 10:49:21.000000000 -0600
> @@ -363,6 +363,9 @@
>                  if (ap->flags & XFSMNT_32BITINODES)
>                          mp->m_flags |= XFS_MOUNT_32BITINODES;
>
> +               if (ap->flags & XFSMNT_IRIXSGID)
> +                       mp->m_flags |= XFS_MOUNT_IRIXSGID;
> +
>                  if (ap->flags & XFSMNT_IOSIZE) {
>                          if (ap->iosizelog > XFS_MAX_IO_LOG ||
>                              ap->iosizelog < XFS_MIN_IO_LOG) {


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