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 <[email protected]>
---
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
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" <[email protected]> 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 <[email protected]>
> > ---
> > 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
[email protected]
http://www.netapp.com
On Thu, 16 Dec 2010 12:49:52 -0500, Trond Myklebust <[email protected]> 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" <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
Any update on this ?
-aneesh
On Thu, 9 Dec 2010 17:05:14 +0530, "Aneesh Kumar K.V" <[email protected]> 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 <[email protected]>
> ---
> 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
>
On Fri, 17 Dec 2010 12:10:38 +0530, "Aneesh Kumar K. V" <[email protected]> wrote:
> On Thu, 16 Dec 2010 12:49:52 -0500, Trond Myklebust <[email protected]> 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" <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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