Patchwork [BUG:2553] nfs: capture attrs of create request in cs->stbuf for later use

login
register
Submitter Anand Avati
Date 2011-03-28 15:00:38
Message ID <20110328150038.GA24592@dev.gluster.com>
Download mbox | patch
Permalink /patch/6621/
State Accepted
Headers show

Comments

Anand Avati - 2011-03-28 15:00:38
when attrs coming in as part of create request contain modes other
than mode, they were getting discarded previously and a setattr
was getting performed on a 0-filled iatt structure. This would
result in EPERM at the access control translator as non-root users
cannot chown a file to uid 0.

Not seen with Linux NFS client as it (very likely) relies upon
auth-unix to set the ownership of the file or sends an explicit
setattr after the create.

Signed-off-by: Anand Avati <avati@gluster.com>
---
 xlators/nfs/server/src/nfs3.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Shehjar Tikoo - 2011-03-28 16:49:54
Reviewed OK...while in a lot of pain...I dont know how this was missed. Please send for release-3.1 too. Thanks

----- Original Message -----
> From: "Anand Avati" <avati@gluster.com>
> To: glusterfs@dev.gluster.com
> Cc: shehjart@gluster.com
> Sent: Monday, March 28, 2011 8:30:38 PM
> Subject: [PATCH BUG:2553] nfs: capture attrs of create request in cs->stbuf for later use
> when attrs coming in as part of create request contain modes other
> than mode, they were getting discarded previously and a setattr
> was getting performed on a 0-filled iatt structure. This would
> result in EPERM at the access control translator as non-root users
> cannot chown a file to uid 0.
> 
> Not seen with Linux NFS client as it (very likely) relies upon
> auth-unix to set the ownership of the file or sends an explicit
> setattr after the create.
> 
> Signed-off-by: Anand Avati <avati@gluster.com>
> ---
> xlators/nfs/server/src/nfs3.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/xlators/nfs/server/src/nfs3.c
> b/xlators/nfs/server/src/nfs3.c
> index e23d4ad..0c2f88b 100644
> --- a/xlators/nfs/server/src/nfs3.c
> +++ b/xlators/nfs/server/src/nfs3.c
> @@ -2512,7 +2512,7 @@ nfs3_create (rpcsvc_request_t *req, struct
> nfs3_fh *dirfh, char *name,
> nfs3_handle_call_state_init (nfs3, cs, req, vol, stat, nfs3err);
> 
> cs->cookieverf = cverf;
> - cs->setattr_valid = nfs3_sattr3_to_setattr_valid (sattr, NULL,
> + cs->setattr_valid = nfs3_sattr3_to_setattr_valid (sattr, &cs->stbuf,
> &cs->mode);
> cs->createmode = mode;
> cs->parent = *dirfh;
> --
> 1.7.1
> 
> 
> --
> ultimate_answer_t
> deep_thought (void)
> {
> sleep (years2secs (7500000));
> return 42;
> }
Shehjar Tikoo - 2011-03-28 16:50:24
----- Original Message -----
> From: "Anand Avati" <avati@gluster.com>
> To: glusterfs@dev.gluster.com
> Cc: shehjart@gluster.com
> Sent: Monday, March 28, 2011 8:30:38 PM
> Subject: [PATCH BUG:2553] nfs: capture attrs of create request in cs->stbuf for later use
> when attrs coming in as part of create request contain modes other
> than mode, they were getting discarded previously and a setattr
> was getting performed on a 0-filled iatt structure. This would
> result in EPERM at the access control translator as non-root users
> cannot chown a file to uid 0.
> 
> Not seen with Linux NFS client as it (very likely) relies upon
> auth-unix to set the ownership of the file or sends an explicit
> setattr after the create.

Yes. It sends a separate setattr NFS request.

> 
> Signed-off-by: Anand Avati <avati@gluster.com>
> ---
> xlators/nfs/server/src/nfs3.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/xlators/nfs/server/src/nfs3.c
> b/xlators/nfs/server/src/nfs3.c
> index e23d4ad..0c2f88b 100644
> --- a/xlators/nfs/server/src/nfs3.c
> +++ b/xlators/nfs/server/src/nfs3.c
> @@ -2512,7 +2512,7 @@ nfs3_create (rpcsvc_request_t *req, struct
> nfs3_fh *dirfh, char *name,
> nfs3_handle_call_state_init (nfs3, cs, req, vol, stat, nfs3err);
> 
> cs->cookieverf = cverf;
> - cs->setattr_valid = nfs3_sattr3_to_setattr_valid (sattr, NULL,
> + cs->setattr_valid = nfs3_sattr3_to_setattr_valid (sattr, &cs->stbuf,
> &cs->mode);
> cs->createmode = mode;
> cs->parent = *dirfh;
> --
> 1.7.1
> 
> 
> --
> ultimate_answer_t
> deep_thought (void)
> {
> sleep (years2secs (7500000));
> return 42;
> }
Gaurav - 2011-03-29 08:21:00
As per RFC in case of EXCLUSIVE create mode, nfs client is not supposed send any attr in create request,
it has to send seperate setattr request for that. Assumption is server will use file attrs mtime field to 
store verf value sent in exclusive create request, so client has to set valid attrs after successful create request.
So cs->stbuf will be invalid in this case, we have to handle it separately.

Regards.
Gaurav

----- Original Message -----
From: "Shehjar Tikoo" <shehjart@gluster.com>
To: "Anand Avati" <avati@gluster.com>
Cc: glusterfs@dev.gluster.com
Sent: Monday, March 28, 2011 10:20:24 PM
Subject: Re: [PATCH BUG:2553] nfs: capture attrs of create request in	cs->stbuf	for later use



----- Original Message -----
> From: "Anand Avati" <avati@gluster.com>
> To: glusterfs@dev.gluster.com
> Cc: shehjart@gluster.com
> Sent: Monday, March 28, 2011 8:30:38 PM
> Subject: [PATCH BUG:2553] nfs: capture attrs of create request in cs->stbuf for later use
> when attrs coming in as part of create request contain modes other
> than mode, they were getting discarded previously and a setattr
> was getting performed on a 0-filled iatt structure. This would
> result in EPERM at the access control translator as non-root users
> cannot chown a file to uid 0.
> 
> Not seen with Linux NFS client as it (very likely) relies upon
> auth-unix to set the ownership of the file or sends an explicit
> setattr after the create.

Yes. It sends a separate setattr NFS request.

> 
> Signed-off-by: Anand Avati <avati@gluster.com>
> ---
> xlators/nfs/server/src/nfs3.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/xlators/nfs/server/src/nfs3.c
> b/xlators/nfs/server/src/nfs3.c
> index e23d4ad..0c2f88b 100644
> --- a/xlators/nfs/server/src/nfs3.c
> +++ b/xlators/nfs/server/src/nfs3.c
> @@ -2512,7 +2512,7 @@ nfs3_create (rpcsvc_request_t *req, struct
> nfs3_fh *dirfh, char *name,
> nfs3_handle_call_state_init (nfs3, cs, req, vol, stat, nfs3err);
> 
> cs->cookieverf = cverf;
> - cs->setattr_valid = nfs3_sattr3_to_setattr_valid (sattr, NULL,
> + cs->setattr_valid = nfs3_sattr3_to_setattr_valid (sattr, &cs->stbuf,
> &cs->mode);
> cs->createmode = mode;
> cs->parent = *dirfh;
> --
> 1.7.1
> 
> 
> --
> ultimate_answer_t
> deep_thought (void)
> {
> sleep (years2secs (7500000));
> return 42;
> }
Shehjar Tikoo - 2011-03-29 08:27:56
Thats actually a different bug because exclusive create itself is implemented incompletely right now. This patch is still needed to extract any attributes sent by the client.

----- Original Message -----
> From: "Gangalwar" <gaurav@gluster.com>
> To: "Shehjar Tikoo" <shehjart@gluster.com>
> Cc: glusterfs@dev.gluster.com, "Anand Avati" <avati@gluster.com>
> Sent: Tuesday, March 29, 2011 1:51:00 PM
> Subject: Re: [PATCH BUG:2553] nfs: capture attrs of create request in cs->stbuf for later use
> As per RFC in case of EXCLUSIVE create mode, nfs client is not
> supposed send any attr in create request,
> it has to send seperate setattr request for that. Assumption is server
> will use file attrs mtime field to
> store verf value sent in exclusive create request, so client has to
> set valid attrs after successful create request.
> So cs->stbuf will be invalid in this case, we have to handle it
> separately.
> 
> Regards.
> Gaurav
> 
> ----- Original Message -----
> From: "Shehjar Tikoo" <shehjart@gluster.com>
> To: "Anand Avati" <avati@gluster.com>
> Cc: glusterfs@dev.gluster.com
> Sent: Monday, March 28, 2011 10:20:24 PM
> Subject: Re: [PATCH BUG:2553] nfs: capture attrs of create request in
> cs->stbuf for later use
> 
> 
> 
> ----- Original Message -----
> > From: "Anand Avati" <avati@gluster.com>
> > To: glusterfs@dev.gluster.com
> > Cc: shehjart@gluster.com
> > Sent: Monday, March 28, 2011 8:30:38 PM
> > Subject: [PATCH BUG:2553] nfs: capture attrs of create request in
> > cs->stbuf for later use
> > when attrs coming in as part of create request contain modes other
> > than mode, they were getting discarded previously and a setattr
> > was getting performed on a 0-filled iatt structure. This would
> > result in EPERM at the access control translator as non-root users
> > cannot chown a file to uid 0.
> >
> > Not seen with Linux NFS client as it (very likely) relies upon
> > auth-unix to set the ownership of the file or sends an explicit
> > setattr after the create.
> 
> Yes. It sends a separate setattr NFS request.
> 
> >
> > Signed-off-by: Anand Avati <avati@gluster.com>
> > ---
> > xlators/nfs/server/src/nfs3.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/xlators/nfs/server/src/nfs3.c
> > b/xlators/nfs/server/src/nfs3.c
> > index e23d4ad..0c2f88b 100644
> > --- a/xlators/nfs/server/src/nfs3.c
> > +++ b/xlators/nfs/server/src/nfs3.c
> > @@ -2512,7 +2512,7 @@ nfs3_create (rpcsvc_request_t *req, struct
> > nfs3_fh *dirfh, char *name,
> > nfs3_handle_call_state_init (nfs3, cs, req, vol, stat, nfs3err);
> >
> > cs->cookieverf = cverf;
> > - cs->setattr_valid = nfs3_sattr3_to_setattr_valid (sattr, NULL,
> > + cs->setattr_valid = nfs3_sattr3_to_setattr_valid (sattr,
> > &cs->stbuf,
> > &cs->mode);
> > cs->createmode = mode;
> > cs->parent = *dirfh;
> > --
> > 1.7.1
> >
> >
> > --
> > ultimate_answer_t
> > deep_thought (void)
> > {
> > sleep (years2secs (7500000));
> > return 42;
> > }
> _______________________________________________
> glusterfs mailing list
> glusterfs@dev.gluster.com
> http://dev.gluster.com/cgi-bin/mailman/listinfo/glusterfs

Patch

diff --git a/xlators/nfs/server/src/nfs3.c b/xlators/nfs/server/src/nfs3.c
index e23d4ad..0c2f88b 100644
--- a/xlators/nfs/server/src/nfs3.c
+++ b/xlators/nfs/server/src/nfs3.c
@@ -2512,7 +2512,7 @@  nfs3_create (rpcsvc_request_t *req, struct nfs3_fh *dirfh, char *name,
         nfs3_handle_call_state_init (nfs3, cs, req, vol, stat, nfs3err);
 
         cs->cookieverf = cverf;
-        cs->setattr_valid = nfs3_sattr3_to_setattr_valid (sattr, NULL,
+        cs->setattr_valid = nfs3_sattr3_to_setattr_valid (sattr, &cs->stbuf,
                                                           &cs->mode);
         cs->createmode = mode;
         cs->parent = *dirfh;