Re: [RFC] Apply the umask in VFS optionally (also POSIX ACL kernel infrastructure)

[Andreas Gruenbacher <agruen@suse.de>]

On Sunday 04 August 2002 16:42, Christoph Hellwig wrote:
> On Sun, Aug 04, 2002 at 03:46:43PM +0200, Andreas Gruenbacher wrote:
> > I believe that (2) is the more reasonable choice in this case, so I
> > propose this patch, which adds the MS_NOUMASK mount option. The flag is
> > set by the file system, if the file system does not want the VFS to apply
> > the umask, after which the file system itself is responsible for applying
> > the umask where appropriate.
>
> In the current XFS trees we have that flag as IS_POSIXACL() and S_POSIXACL
> inode flag. (And I think some of your 2.4 patches do the same).  After some
> thinking a per-superblock flag looks best to me, but instead of naming it
> MS_NOUMASK I'd really, really prefer MS_POSIXACL.

Well, the previous macro was IS_POSIX_ACL(inode); it was testing a super block 
flag. We had an additional s_xattr_flags in the super block:


--- linux/include/linux/fs.h.orig	Thu Aug  1 11:17:45 2002
+++ linux/include/linux/fs.h	Thu Aug  1 11:32:22 2002
@@ -117,6 +117,21 @@
 #define MS_NOUSER	(1<<31)
 
 /*
+ * These are the super block s_ext_attr_flags
+ */
+#define XATTR_MNT_FLAG_ALL		1	/* Extended attributes */
+#define XATTR_MNT_FLAG_USER		1	/* Extended user attributes */
+#define XATTR_MNT_FLAG_POSIX_ACL	2	/* Access Control Lists */
+
+#define __IS_XATTR_FLG(inode,flg) \
+	((inode)->i_sb && \
+	 (inode)->i_sb->s_xattr_flags & (XATTR_MNT_FLAG_ ## flg))
+
+#define IS_XATTR(inode)		__IS_XATTR_FLG(inode, ALL)
+#define IS_XATTR_USER(inode)	__IS_XATTR_FLG(inode, XATTR_USER)
+#define IS_POSIX_ACL(inode)	__IS_XATTR_FLG(inode, POSIX_ACL)
+
+/*
  * Superblock flags that can be altered by MS_REMOUNT
  */
 #define MS_RMT_MASK	(MS_RDONLY|MS_SYNCHRONOUS|MS_MANDLOCK|MS_NOATIME|\
@@ -785,6 +800,7 @@
 	struct super_operations	*s_op;
 	struct dquot_operations	*dq_op;
 	unsigned long		s_flags;
+	unsigned long		s_xattr_flags;
 	unsigned long		s_magic;
 	struct dentry		*s_root;
 	struct rw_semaphore	s_umount;


The MS_NOUMASK flag is in s_flags, and so doesn't require an additinal field. 
In the Ext2/Ext3 patch that I'm working on, there is a per-filesystem mount 
flag for POSIX ACLs (and also for USER EAs), which is being synchronized with 
MS_NOUMASK.

> Rationale:
>
> (1) there is not much other reasoning why we shouldn't apply the umask
> (2) this way lowlevel filesystem drivers can enabled/disable acl
>     per-mountpoint or even on-the-fly (if remount implements it)

>From the point of the VFS it's just `don't apply the umask'; the VFS doesn't 
need to care about POSIX ACLs otherwise. (And who knows if there won't be 
another case where this is needed?) The low-level filesystem can still 
enable/disable ACLs per mount point or even per inode, it only needs to take 
care of the umask itself.

> > Finally, I have a question related to this. We had a bug with kernel
> > tasks, which don't have a umask associated with them (nfsd in
> > particular). Should kernel tasks that create files be required to have a
> > valid fs_struct (which includes the umask), or should this be special
> > cased in file systems?
>
> Anything that deals with files shall have a valid fs_struct.  The number
> of in-kernel threads that are supposed to deal with files is extremly
> low (only in-kernel fileservers like nfsd, tux or the now dead khttpd)
> and not worth workarounds in filesystem code.  Not to mention it is much
> more elegant.

That sounds very reasonable.

Regards,
Andreas.

------------------------------------------------------------------
 Andreas Gruenbacher                                SuSE Linux AG
 mailto:agruen@suse.de                     Deutschherrnstr. 15-19
 http://www.suse.de/                   D-90429 Nuernberg, Germany