[PATCH] nfs: Set MS_POSIXACL always

[PATCH] nfs: Set MS_POSIXACL always

[Aneesh Kumar K.V]

We want to skip VFS applying mode for NFS. So set MS_POSIXACL always
and selectively use umask. Ideally we would want to use umask only
when we don't have inheritable ACEs set. But NFS currently don't
allow to send umask to the server. So this is best what we can do
and this is consistent with NFSv3

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/nfs/dir.c      |    3 +--
 fs/nfs/nfs4proc.c |    5 +++++
 fs/nfs/super.c    |   10 ++++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 8ea4a41..070f368 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1361,8 +1361,7 @@ static struct dentry *nfs_atomic_lookup(struct inode *dir, struct dentry *dentry
 	if (nd->flags & LOOKUP_CREATE) {
 		attr.ia_mode = nd->intent.open.create_mode;
 		attr.ia_valid = ATTR_MODE;
-		if (!IS_POSIXACL(dir))
-			attr.ia_mode &= ~current_umask();
+		attr.ia_mode &= ~current_umask();
 	} else {
 		open_flags &= ~(O_EXCL | O_CREAT);
 		attr.ia_valid = 0;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6a653ff..c57b4e0 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2486,6 +2486,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 		path = &ctx->path;
 		fmode = ctx->mode;
 	}
+	sattr->ia_mode &= ~current_umask();
 	state = nfs4_do_open(dir, path, fmode, flags, sattr, cred);
 	d_drop(dentry);
 	if (IS_ERR(state)) {
@@ -2816,6 +2817,8 @@ static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
 {
 	struct nfs4_exception exception = { };
 	int err;
+
+	sattr->ia_mode &= ~current_umask();
 	do {
 		err = nfs4_handle_exception(NFS_SERVER(dir),
 				_nfs4_proc_mkdir(dir, dentry, sattr),
@@ -2916,6 +2919,8 @@ static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
 {
 	struct nfs4_exception exception = { };
 	int err;
+
+	sattr->ia_mode &= ~current_umask();
 	do {
 		err = nfs4_handle_exception(NFS_SERVER(dir),
 				_nfs4_proc_mknod(dir, dentry, sattr, rdev),
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 3c04504..e57e670 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2498,6 +2498,11 @@ static void nfs4_clone_super(struct super_block *sb,
 	sb->s_maxbytes = old_sb->s_maxbytes;
 	sb->s_time_gran = 1;
 	sb->s_op = old_sb->s_op;
+	/*
+	 * The VFS shouldn't apply the umask to mode bits. We will do
+	 * so ourselves when necessary.
+	 */
+	sb->s_flags  |= MS_POSIXACL;
  	nfs_initialise_sb(sb);
 }
 
@@ -2508,6 +2513,11 @@ static void nfs4_fill_super(struct super_block *sb)
 {
 	sb->s_time_gran = 1;
 	sb->s_op = &nfs4_sops;
+	/*
+	 * The VFS shouldn't apply the umask to mode bits. We will do
+	 * so ourselves when necessary.
+	 */
+	sb->s_flags  |= MS_POSIXACL;
 	nfs_initialise_sb(sb);
 }
 
-- 
1.7.1

Re: [PATCH] nfs: Set MS_POSIXACL always

[Aneesh Kumar K. V]

Any update on this ?

-aneesh

On Thu,  9 Dec 2010 17:05:14 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> We want to skip VFS applying mode for NFS. So set MS_POSIXACL always
> and selectively use umask. Ideally we would want to use umask only
> when we don't have inheritable ACEs set. But NFS currently don't
> allow to send umask to the server. So this is best what we can do
> and this is consistent with NFSv3
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  fs/nfs/dir.c      |    3 +--
>  fs/nfs/nfs4proc.c |    5 +++++
>  fs/nfs/super.c    |   10 ++++++++++
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 8ea4a41..070f368 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1361,8 +1361,7 @@ static struct dentry *nfs_atomic_lookup(struct inode *dir, struct dentry *dentry
>  	if (nd->flags & LOOKUP_CREATE) {
>  		attr.ia_mode = nd->intent.open.create_mode;
>  		attr.ia_valid = ATTR_MODE;
> -		if (!IS_POSIXACL(dir))
> -			attr.ia_mode &= ~current_umask();
> +		attr.ia_mode &= ~current_umask();
>  	} else {
>  		open_flags &= ~(O_EXCL | O_CREAT);
>  		attr.ia_valid = 0;
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6a653ff..c57b4e0 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2486,6 +2486,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
>  		path = &ctx->path;
>  		fmode = ctx->mode;
>  	}
> +	sattr->ia_mode &= ~current_umask();
>  	state = nfs4_do_open(dir, path, fmode, flags, sattr, cred);
>  	d_drop(dentry);
>  	if (IS_ERR(state)) {
> @@ -2816,6 +2817,8 @@ static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
>  {
>  	struct nfs4_exception exception = { };
>  	int err;
> +
> +	sattr->ia_mode &= ~current_umask();
>  	do {
>  		err = nfs4_handle_exception(NFS_SERVER(dir),
>  				_nfs4_proc_mkdir(dir, dentry, sattr),
> @@ -2916,6 +2919,8 @@ static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
>  {
>  	struct nfs4_exception exception = { };
>  	int err;
> +
> +	sattr->ia_mode &= ~current_umask();
>  	do {
>  		err = nfs4_handle_exception(NFS_SERVER(dir),
>  				_nfs4_proc_mknod(dir, dentry, sattr, rdev),
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 3c04504..e57e670 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2498,6 +2498,11 @@ static void nfs4_clone_super(struct super_block *sb,
>  	sb->s_maxbytes = old_sb->s_maxbytes;
>  	sb->s_time_gran = 1;
>  	sb->s_op = old_sb->s_op;
> +	/*
> +	 * The VFS shouldn't apply the umask to mode bits. We will do
> +	 * so ourselves when necessary.
> +	 */
> +	sb->s_flags  |= MS_POSIXACL;
>   	nfs_initialise_sb(sb);
>  }
> 
> @@ -2508,6 +2513,11 @@ static void nfs4_fill_super(struct super_block *sb)
>  {
>  	sb->s_time_gran = 1;
>  	sb->s_op = &nfs4_sops;
> +	/*
> +	 * The VFS shouldn't apply the umask to mode bits. We will do
> +	 * so ourselves when necessary.
> +	 */
> +	sb->s_flags  |= MS_POSIXACL;
>  	nfs_initialise_sb(sb);
>  }
> 
> -- 
> 1.7.1
> 

Re: [PATCH] nfs: Set MS_POSIXACL always

[Trond Myklebust]

On Thu, 2010-12-16 at 22:45 +0530, Aneesh Kumar K. V wrote:
> Any update on this ?
> 
> -aneesh
> 
> On Thu,  9 Dec 2010 17:05:14 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > We want to skip VFS applying mode for NFS. So set MS_POSIXACL always
> > and selectively use umask. Ideally we would want to use umask only
> > when we don't have inheritable ACEs set. But NFS currently don't
> > allow to send umask to the server. So this is best what we can do
> > and this is consistent with NFSv3
> > 
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > ---
> >  fs/nfs/dir.c      |    3 +--
> >  fs/nfs/nfs4proc.c |    5 +++++
> >  fs/nfs/super.c    |   10 ++++++++++
> >  3 files changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 3c04504..e57e670 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2508,6 +2513,11 @@ static void nfs4_fill_super(struct super_block *sb)
> >  {
> >  	sb->s_time_gran = 1;
> >  	sb->s_op = &nfs4_sops;
> > +	/*
> > +	 * The VFS shouldn't apply the umask to mode bits. We will do
> > +	 * so ourselves when necessary.
> > +	 */
> > +	sb->s_flags  |= MS_POSIXACL;
> >  	nfs_initialise_sb(sb);
> >  }

Won't this end up possibly turning on ACL checking in
acl_permission_check()?

Cheers
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH] nfs: Set MS_POSIXACL always

[Aneesh Kumar K. V]

On Thu, 16 Dec 2010 12:49:52 -0500, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> On Thu, 2010-12-16 at 22:45 +0530, Aneesh Kumar K. V wrote:
> > Any update on this ?
> > 
> > -aneesh
> > 
> > On Thu,  9 Dec 2010 17:05:14 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > > We want to skip VFS applying mode for NFS. So set MS_POSIXACL always
> > > and selectively use umask. Ideally we would want to use umask only
> > > when we don't have inheritable ACEs set. But NFS currently don't
> > > allow to send umask to the server. So this is best what we can do
> > > and this is consistent with NFSv3
> > > 
> > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > > ---
> > >  fs/nfs/dir.c      |    3 +--
> > >  fs/nfs/nfs4proc.c |    5 +++++
> > >  fs/nfs/super.c    |   10 ++++++++++
> > >  3 files changed, 16 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > index 3c04504..e57e670 100644
> > > --- a/fs/nfs/super.c
> > > +++ b/fs/nfs/super.c
> > > @@ -2508,6 +2513,11 @@ static void nfs4_fill_super(struct super_block *sb)
> > >  {
> > >  	sb->s_time_gran = 1;
> > >  	sb->s_op = &nfs4_sops;
> > > +	/*
> > > +	 * The VFS shouldn't apply the umask to mode bits. We will do
> > > +	 * so ourselves when necessary.
> > > +	 */
> > > +	sb->s_flags  |= MS_POSIXACL;
> > >  	nfs_initialise_sb(sb);
> > >  }
> 
> Won't this end up possibly turning on ACL checking in
> acl_permission_check()?

acl_permission_check get called only when we don't define an
inode->permission callback. In case of nfs client we do define 
nfs_permission callback. In case we don't define a access NFS proto
callback we are still ok because in that case we want mode based
validation and we do pass check_acl as NULL 

-aneesh

Re: [PATCH] nfs: Set MS_POSIXACL always

[Aneesh Kumar K. V]

On Fri, 17 Dec 2010 12:10:38 +0530, "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> On Thu, 16 Dec 2010 12:49:52 -0500, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> > On Thu, 2010-12-16 at 22:45 +0530, Aneesh Kumar K. V wrote:
> > > Any update on this ?
> > > 
> > > -aneesh
> > > 
> > > On Thu,  9 Dec 2010 17:05:14 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > > > We want to skip VFS applying mode for NFS. So set MS_POSIXACL always
> > > > and selectively use umask. Ideally we would want to use umask only
> > > > when we don't have inheritable ACEs set. But NFS currently don't
> > > > allow to send umask to the server. So this is best what we can do
> > > > and this is consistent with NFSv3
> > > > 
> > > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > > > ---
> > > >  fs/nfs/dir.c      |    3 +--
> > > >  fs/nfs/nfs4proc.c |    5 +++++
> > > >  fs/nfs/super.c    |   10 ++++++++++
> > > >  3 files changed, 16 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > > index 3c04504..e57e670 100644
> > > > --- a/fs/nfs/super.c
> > > > +++ b/fs/nfs/super.c
> > > > @@ -2508,6 +2513,11 @@ static void nfs4_fill_super(struct super_block *sb)
> > > >  {
> > > >  	sb->s_time_gran = 1;
> > > >  	sb->s_op = &nfs4_sops;
> > > > +	/*
> > > > +	 * The VFS shouldn't apply the umask to mode bits. We will do
> > > > +	 * so ourselves when necessary.
> > > > +	 */
> > > > +	sb->s_flags  |= MS_POSIXACL;
> > > >  	nfs_initialise_sb(sb);
> > > >  }
> > 
> > Won't this end up possibly turning on ACL checking in
> > acl_permission_check()?
> 
> acl_permission_check get called only when we don't define an
> inode->permission callback. In case of nfs client we do define 
> nfs_permission callback. In case we don't define a access NFS proto
> callback we are still ok because in that case we want mode based
> validation and we do pass check_acl as NULL 
> 

Can we get this upstream ?

-aneesh