All of lore.kernel.org
 help / color / mirror / Atom feed
* Open with O_CREAT flag set fails to open existing files on non writable directories
@ 2011-04-20 12:09 Sachin Prabhu
  2011-04-20 15:23 ` J. Bruce Fields
  0 siblings, 1 reply; 3+ messages in thread
From: Sachin Prabhu @ 2011-04-20 12:09 UTC (permalink / raw)
  To: Linux NFS Mailing List

An open on a NFS4 share using the O_CREAT flag on an existing file for
which we have permissions to open but contained in a directory with no
write permissions will fail with EACCES.

A tcpdump shows that the client had set the open mode to UNCHECKED which
indicates that the file should be created if it doesn't exist and
encountering an existing flag is not an error. Since in this case the
file exists and can be opened by the user, the NFS server is wrong in
attempting to check create permissions on the parent directory.

The patch adds a conditional statement to check for create permissions
only if the file doesn't exist.

Signed-off-by: Sachin S. Prabhu <sprabhu@redhat.com>

diff -up linux-2.6/fs/nfsd/vfs.c.bz683372 linux-2.6/fs/nfsd/vfs.c
--- linux-2.6/fs/nfsd/vfs.c.bz683372	2011-04-20 13:03:54.021040329 +0100
+++ linux-2.6/fs/nfsd/vfs.c	2011-04-20 13:05:21.551858218 +0100
@@ -1363,7 +1363,7 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
 		goto out;
 	if (!(iap->ia_valid & ATTR_MODE))
 		iap->ia_mode = 0;
-	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
+	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_EXEC);
 	if (err)
 		goto out;
 
@@ -1385,6 +1385,13 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
 	if (IS_ERR(dchild))
 		goto out_nfserr;
 
+	/* If file doesn't exist, check for permissions to create one */
+	if (!dchild->d_inode) {
+		err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
+		if (err)
+			goto out;
+	}
+
 	err = fh_compose(resfhp, fhp->fh_export, dchild, fhp);
 	if (err)
 		goto out;



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Open with O_CREAT flag set fails to open existing files on non writable directories
  2011-04-20 12:09 Sachin Prabhu
@ 2011-04-20 15:23 ` J. Bruce Fields
  0 siblings, 0 replies; 3+ messages in thread
From: J. Bruce Fields @ 2011-04-20 15:23 UTC (permalink / raw)
  To: Sachin Prabhu; +Cc: Linux NFS Mailing List

On Wed, Apr 20, 2011 at 01:09:35PM +0100, Sachin Prabhu wrote:
> An open on a NFS4 share using the O_CREAT flag on an existing file for
> which we have permissions to open but contained in a directory with no
> write permissions will fail with EACCES.
> 
> A tcpdump shows that the client had set the open mode to UNCHECKED which
> indicates that the file should be created if it doesn't exist and
> encountering an existing flag is not an error. Since in this case the
> file exists and can be opened by the user, the NFS server is wrong in
> attempting to check create permissions on the parent directory.
> 
> The patch adds a conditional statement to check for create permissions
> only if the file doesn't exist.

That looks correct to me, thanks.

It's a bad bug, but appears to be one we've lived with a long time, so
I'm a bit up in the air whether to submit now for 2.6.39 or to queue up
for 2.6.40.

--b.

> 
> Signed-off-by: Sachin S. Prabhu <sprabhu@redhat.com>
> 
> diff -up linux-2.6/fs/nfsd/vfs.c.bz683372 linux-2.6/fs/nfsd/vfs.c
> --- linux-2.6/fs/nfsd/vfs.c.bz683372	2011-04-20 13:03:54.021040329 +0100
> +++ linux-2.6/fs/nfsd/vfs.c	2011-04-20 13:05:21.551858218 +0100
> @@ -1363,7 +1363,7 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
>  		goto out;
>  	if (!(iap->ia_valid & ATTR_MODE))
>  		iap->ia_mode = 0;
> -	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
> +	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_EXEC);
>  	if (err)
>  		goto out;
>  
> @@ -1385,6 +1385,13 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
>  	if (IS_ERR(dchild))
>  		goto out_nfserr;
>  
> +	/* If file doesn't exist, check for permissions to create one */
> +	if (!dchild->d_inode) {
> +		err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
> +		if (err)
> +			goto out;
> +	}
> +
>  	err = fh_compose(resfhp, fhp->fh_export, dchild, fhp);
>  	if (err)
>  		goto out;
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Open with O_CREAT flag set fails to open existing files on non writable directories
@ 2011-04-21  7:24 Rik Theys
  0 siblings, 0 replies; 3+ messages in thread
From: Rik Theys @ 2011-04-21  7:24 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

Hi,

 >> The patch adds a conditional statement to check for create permissions
 >> only if the file doesn't exist.

 > That looks correct to me, thanks.

 > It's a bad bug, but appears to be one we've lived with a long time, so
 > I'm a bit up in the air whether to submit now for 2.6.39 or to queue up
 > for 2.6.40.

As an administrator who is hit by this bug on a daily basis, I would 
like it to reach 2.6.39. But as I filed the bug, I guess I'm biased :-).

Distributions would like to see this accepted upstream before they 
consider backporting the fix to their current stable releases.


Regards,

Rik

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-04-21  7:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-21  7:24 Open with O_CREAT flag set fails to open existing files on non writable directories Rik Theys
  -- strict thread matches above, loose matches on Subject: below --
2011-04-20 12:09 Sachin Prabhu
2011-04-20 15:23 ` J. Bruce Fields

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.