diff mbox

[Lucid] SRU: nfsd4: permit read opens of executable-only files

Message ID 4EEA5926.9090500@canonical.com
State New
Headers show

Commit Message

Chris J Arges Dec. 15, 2011, 8:31 p.m. UTC
SRU Justification

Impact:

NFSv4 mount point does not allow binary files to run when permissions
are set to 111.

Fix:

Upstream commit a043226bc140a2c1dde162246d68a67e5043e6b2.
Applies with minor conflicts (include file is in a different directory).

Testcase:
Mount a directory using nfsv4. Inside that directory place a binary with
---x--x--x permissions. Try to execute the binary. Without this patch it
will not execute.

Notes:
This patch fixes this issue, but requires support from the nfs client as
well. The Lucid version of the client works as expected.

Thanks,
--chris j arges

Comments

Herton Ronaldo Krzesinski Dec. 16, 2011, 11:46 a.m. UTC | #1
On Thu, Dec 15, 2011 at 02:31:34PM -0600, Chris J Arges wrote:
> SRU Justification
> 
> Impact:
> 
> NFSv4 mount point does not allow binary files to run when permissions
> are set to 111.
> 
> Fix:
> 
> Upstream commit a043226bc140a2c1dde162246d68a67e5043e6b2.
> Applies with minor conflicts (include file is in a different directory).
> 
> Testcase:
> Mount a directory using nfsv4. Inside that directory place a binary with
> ---x--x--x permissions. Try to execute the binary. Without this patch it
> will not execute.
> 
> Notes:
> This patch fixes this issue, but requires support from the nfs client as
> well. The Lucid version of the client works as expected.
> 
> Thanks,
> --chris j arges

> From 88c0bdee31a467bb55a3f812b6ae63dae4330a79 Mon Sep 17 00:00:00 2001
> From: "J. Bruce Fields" <bfields@redhat.com>
> Date: Thu, 25 Aug 2011 10:48:39 -0400
> Subject: [PATCH] nfsd4: permit read opens of executable-only files
> 
> A client that wants to execute a file must be able to read it.  Read
> opens over nfs are therefore implicitly allowed for executable files
> even when those files are not readable.
> 
> NFSv2/v3 get this right by using a passed-in NFSD_MAY_OWNER_OVERRIDE on
> read requests, but NFSv4 has gotten this wrong ever since
> dc730e173785e29b297aa605786c94adaffe2544 "nfsd4: fix owner-override on
> open", when we realized that the file owner shouldn't override
> permissions on non-reclaim NFSv4 opens.
> 
> So we can't use NFSD_MAY_OWNER_OVERRIDE to tell nfsd_permission to allow
> reads of executable files.
> 
> So, do the same thing we do whenever we encounter another weird NFS
> permission nit: define yet another NFSD_MAY_* flag.
> 
> The industry's future standardization on 128-bit processors will be
> motivated primarily by the need for integers with enough bits for all
> the NFSD_MAY_* flags.
> 
> Reported-by: Leonardo Borda <leonardoborda@gmail.com>
> Cc: stable@kernel.org
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> (cherry picked from commit a043226bc140a2c1dde162246d68a67e5043e6b2)
> 
> BugLink: http://bugs.launchpad.net/bugs/833300

You forgot to add your Sign-off, no big problem though:

Acked-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>

> 
> Conflicts:
> 
> 	fs/nfsd/vfs.h
> ---
>  fs/nfsd/nfs4proc.c        |    2 ++
>  fs/nfsd/vfs.c             |    3 ++-
>  include/linux/nfsd/nfsd.h |    1 +
>  3 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 01d83a5..f865116 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -166,6 +166,8 @@ do_open_permission(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfs
>  		!(open->op_share_access & NFS4_SHARE_ACCESS_WRITE))
>  		return nfserr_inval;
>  
> +	accmode |= NFSD_MAY_READ_IF_EXEC;
> +
>  	if (open->op_share_access & NFS4_SHARE_ACCESS_READ)
>  		accmode |= NFSD_MAY_READ;
>  	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE)
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 2e09588..494350b 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -2118,7 +2118,8 @@ nfsd_permission(struct svc_rqst *rqstp, struct svc_export *exp,
>  
>  	/* Allow read access to binaries even when mode 111 */
>  	if (err == -EACCES && S_ISREG(inode->i_mode) &&
> -	    acc == (NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE))
> +	     (acc == (NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE) ||
> +	      acc == (NFSD_MAY_READ | NFSD_MAY_READ_IF_EXEC)))
>  		err = inode_permission(inode, MAY_EXEC);
>  	if (err)
>  		goto nfsd_out;
> diff --git a/include/linux/nfsd/nfsd.h b/include/linux/nfsd/nfsd.h
> index 510ffdd..3436ec6 100644
> --- a/include/linux/nfsd/nfsd.h
> +++ b/include/linux/nfsd/nfsd.h
> @@ -38,6 +38,7 @@
>  #define NFSD_MAY_OWNER_OVERRIDE	64
>  #define NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
>  #define NFSD_MAY_BYPASS_GSS_ON_ROOT 256
> +#define NFSD_MAY_READ_IF_EXEC  2048
>  
>  #define NFSD_MAY_CREATE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE)
>  #define NFSD_MAY_REMOVE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)
> -- 
> 1.7.5.4
> 

> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Tim Gardner Dec. 16, 2011, 3:48 p.m. UTC | #2
On 12/15/2011 01:31 PM, Chris J Arges wrote:
> SRU Justification
>
> Impact:
>
> NFSv4 mount point does not allow binary files to run when permissions
> are set to 111.
>
> Fix:
>
> Upstream commit a043226bc140a2c1dde162246d68a67e5043e6b2.
> Applies with minor conflicts (include file is in a different directory).
>
> Testcase:
> Mount a directory using nfsv4. Inside that directory place a binary with
> ---x--x--x permissions. Try to execute the binary. Without this patch it
> will not execute.
>
> Notes:
> This patch fixes this issue, but requires support from the nfs client as
> well. The Lucid version of the client works as expected.
>
> Thanks,
> --chris j arges
>

If we're gonna provide fixes for Lucid and Natty, then we'd better do so 
for Maverick.
diff mbox

Patch

From 88c0bdee31a467bb55a3f812b6ae63dae4330a79 Mon Sep 17 00:00:00 2001
From: "J. Bruce Fields" <bfields@redhat.com>
Date: Thu, 25 Aug 2011 10:48:39 -0400
Subject: [PATCH] nfsd4: permit read opens of executable-only files

A client that wants to execute a file must be able to read it.  Read
opens over nfs are therefore implicitly allowed for executable files
even when those files are not readable.

NFSv2/v3 get this right by using a passed-in NFSD_MAY_OWNER_OVERRIDE on
read requests, but NFSv4 has gotten this wrong ever since
dc730e173785e29b297aa605786c94adaffe2544 "nfsd4: fix owner-override on
open", when we realized that the file owner shouldn't override
permissions on non-reclaim NFSv4 opens.

So we can't use NFSD_MAY_OWNER_OVERRIDE to tell nfsd_permission to allow
reads of executable files.

So, do the same thing we do whenever we encounter another weird NFS
permission nit: define yet another NFSD_MAY_* flag.

The industry's future standardization on 128-bit processors will be
motivated primarily by the need for integers with enough bits for all
the NFSD_MAY_* flags.

Reported-by: Leonardo Borda <leonardoborda@gmail.com>
Cc: stable@kernel.org
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
(cherry picked from commit a043226bc140a2c1dde162246d68a67e5043e6b2)

BugLink: http://bugs.launchpad.net/bugs/833300

Conflicts:

	fs/nfsd/vfs.h
---
 fs/nfsd/nfs4proc.c        |    2 ++
 fs/nfsd/vfs.c             |    3 ++-
 include/linux/nfsd/nfsd.h |    1 +
 3 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 01d83a5..f865116 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -166,6 +166,8 @@  do_open_permission(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfs
 		!(open->op_share_access & NFS4_SHARE_ACCESS_WRITE))
 		return nfserr_inval;
 
+	accmode |= NFSD_MAY_READ_IF_EXEC;
+
 	if (open->op_share_access & NFS4_SHARE_ACCESS_READ)
 		accmode |= NFSD_MAY_READ;
 	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 2e09588..494350b 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -2118,7 +2118,8 @@  nfsd_permission(struct svc_rqst *rqstp, struct svc_export *exp,
 
 	/* Allow read access to binaries even when mode 111 */
 	if (err == -EACCES && S_ISREG(inode->i_mode) &&
-	    acc == (NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE))
+	     (acc == (NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE) ||
+	      acc == (NFSD_MAY_READ | NFSD_MAY_READ_IF_EXEC)))
 		err = inode_permission(inode, MAY_EXEC);
 	if (err)
 		goto nfsd_out;
diff --git a/include/linux/nfsd/nfsd.h b/include/linux/nfsd/nfsd.h
index 510ffdd..3436ec6 100644
--- a/include/linux/nfsd/nfsd.h
+++ b/include/linux/nfsd/nfsd.h
@@ -38,6 +38,7 @@ 
 #define NFSD_MAY_OWNER_OVERRIDE	64
 #define NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
 #define NFSD_MAY_BYPASS_GSS_ON_ROOT 256
+#define NFSD_MAY_READ_IF_EXEC  2048
 
 #define NFSD_MAY_CREATE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE)
 #define NFSD_MAY_REMOVE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)
-- 
1.7.5.4