xfs
[Top] [All Lists]

TAKE 820248 - application of default ACLs are being wrongly affected by

To: tes@xxxxxxxxxxxxxxxxxxxx
Subject: TAKE 820248 - application of default ACLs are being wrongly affected by umask
From: pv@xxxxxxxxxxxxxxxxxxxxxx (tes@xxxxxxxxxxxx)
Date: Mon, 16 Apr 2001 22:30:04 -0700 (PDT)
Cc: linux-xfs@xxxxxxxxxxx
Reply-to: sgi.bugs.xfs@xxxxxxxxxxxxxxxxx
Sender: owner-linux-xfs@xxxxxxxxxxx
 Submitter : tes                      *Status : closed                      
 Assigned Engineer : tes              *Fixed By : tes                       
*Fixed By Domain : engr               *Closed Date : 04/16/01               
 Priority : 3                         *Modified Date : 04/16/01             
*Modified User : tes                  *Modified User Domain : engr          
*Fix Description :
From: timothy shimmin <tes@xxxxxxxxxxxxxxxxxxxxxxx> (TAKE)
Date: Apr 16 2001 10:30:04PM
[pvnews version: 1.71]
----------------------------

This TAKE modifies the non-xfs files of:
  linux/include/linux/fs.h
  linux/fs/namei.c

I needed to modify namie.c so that the umask would not
unconditionally be applied to the mode - otherwise the
default ACL has no chance of being applied correctly.
Andreas G. did the same thing for his linux/ext2 ACL patch.

I don't know if this is really urgent enough to go into 1.0 .
I would certainly like it go thru QA for a bit and others may
have some better suggestions for the fix.

--Tim

Date:  Mon Apr 16 22:16:51 PDT 2001
Workarea:  snort.melbourne.sgi.com:/diskb/build4/tes/slinx-xfs-acl

The following file(s) were checked into:
  bonnie.engr.sgi.com:/isms/slinx/2.4.x-xfs


Modid:  2.4.x-xfs:slinx:92683a
linux/include/linux/fs.h - 1.89
        - pv#820248; rv:ajag@engr;
          Add IS_POSIX_ACL() macro and acl_flag to the super block.

linux/fs/namei.c - 1.27
        - pv#820248; rv:ajag@engr
          Don't apply umask if ACLs turned on for FS.
          This is done like Andreas Gruenbacher's ACL patch.

linux/fs/xfs/linux/xfs_super.c - 1.117
        - pv#820248; rv:ajag@engr
          Turn on the acl flag for xfs.

linux/fs/xfs/linux/xfs_iops.c - 1.104
        - pv#820248; rv:ajag@engr
          Modify linvfs_common_cr() so that if a default acl does
          not exist for the parent inode (using _ACL_GET_DEFAULT())
          then apply the umask. Otherwise use the default acl for
          _ACL_INHERIT().

cmd/xfstests/051.out - 1.6
        - Updated for 051 change for pv#820248.

cmd/xfstests/051 - 1.6
        - Test default ACLs with particular umasks instead of
          relying on a default umask.
          Updated for pv#820248.

linux/fs/xfs/xfs_acl.h - 1.4
        - pv#820248; rv:ajag@engr;
          Fix up the parameters for _ACL_INHERIT and add _ACL_GET_DEFAULT.

linux/fs/xfs/xfs_acl.c - 1.4
        - pv#820248; rv:ajag@engr;
          Change xfs_acl_inherit() to take the parent's default acl as
          a parameter instead of getting it from the parent's vnode.
          This is done because in linvfs_common_cr() we will already have
          gotten the default ACL and so we should use it instead of getting
          it again.
          ALSO, fix the bug where the group permission of the mode was not
          and'ed in - it was just overwriting the group permission.
Description :
Bug discovered by jtrostel@xxxxxxxxxxx
Basically the umask is being used in the case where
the default ACL should be used.

Bugifying the email :)

========================================
Date: Thu, 29 Mar 2001 17:40:57 +0200
From: Carlos Gamboa Dos Santos <Carlos.Gamboa@xxxxxx>
To: Andrew Gildfind <ajag@xxxxxxxxxxxxxxxxxxxxxxx>
CC: linux-xfs@xxxxxxxxxxx
Subject: Re: Problems with ACL inheritance and chacl

Hi...

Andrew Gildfind wrote:
>
> On Tue, Mar 27, 2001 at 10:51:42AM -0500, John Trostel wrote:
> > I am seeing some strange behavior with both ACL inheritance and chacl
> > operation.  Does anyone else see this or is my build faulty?
> >   
> > I started with a freshly formatted xfs partition on /mnt/xfs_part.
> > 
> > I set the access, default and mask ACLs for this parition as follows:
> >                        
> > [...]
> > 
> > Not good... Shouldn't this get the default ACLs u::rwx,g::rwx,o::rwx ?
> 
> I'm not precisely sure of the semantics of ACL inheritance, but I noticed
> when playing around with this that the inherited ACL was modified by
> different umasks... I'll have to look into this further.

We saw the same behaviour with our builts. After a few tests, we confirmed that
"umask" was changing the active mask of the ACL and therefore limiting the real
accesses.
  
But... shouldn't "umask" be ignored in ACL where a mask is present?
Otherwise it seems that the flexibility of the ACLs is lost...
==============================================================


====================================================
To: John Trostel <jtrostel@xxxxxxxxxx> 
Subject: Re: Problems with ACL inheritance and chacl & maybe a BUG 
From: Timothy Shimmin <tes@xxxxxxxxxxxxxxxxxxxxxxx> 
Date: Tue, 3 Apr 2001 17:47:55 +1000 

Hi,

After looking at the Revised section 5 in the withdrawn
ACL standard, 5.3.1.2 states how a newly created file will
obtain its ACLs.
And indeed, if the ACLs are turned on and a file is created
in a directory which has a default ACL, then the umask
is not supposed to be used at all - only the default ACL and
the operation's mode are to be used.
The rational for this is given in section B.23.5 where it
actually explicitly says that the umask is not to be used.

Looking at the code in fs/xfs/xfs_acl.c and xfs_acl_inherit()
it is doing the right thing.
However, by this stage of setting up the acl the vap->va_mode
has already been changed by the umask !
So there is our bug.

In fs/namei.c (vfs_create(), vfs_mknod() and vfs_mkdir())
it updates the mode with the umask (before we get to see the
mode argument).
There needs to be a test on the directory to see if it
has a default ACL before updating the mode with the umask.

Looking at Andreas' ACL patch, he has in fact patched fs/namei.c
for each case of where umask is used.
If the inode's fs has ACLs turned on
  (inode)->i_sb->s_ext_attr_flags & EXT_ATTR_FLAG_POSIX_ACL
then he doesn't use the umask.
Then in the ACL code, he uses the umask if the directory
doesn't have a default ACL. 
============================================

Plan to do a similar fix as Andreas G. did.

<Prev in Thread] Current Thread [Next in Thread>
  • TAKE 820248 - application of default ACLs are being wrongly affected by umask, tes@xxxxxxxxxxxx <=