From: Miklos Szeredi <mszeredi@suse.cz>

For execute permission on a regular files we need to check if file has
any execute bits at all, regardless of capabilites.

This check is normally performed by generic_permission() but was also
added to the case when the filesystem defines its own ->permission()
method.  In the latter case the filesystem should be responsible for
performing this check.

So create a helper function exec_permission() that checks returns
-EACCESS if MAY_EXEC is present, the inode is a regular file and no
execute bits are present in inode->i_mode.

Call this function from filesystems which don't call
generic_permission() from their ->permission() methods and which
aren't already performing this check.  Also remove the check from
dentry_permission().

The new code should be equivalent to the old.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/cifs/cifsfs.c      |    2 +-
 fs/coda/dir.c         |    4 ++++
 fs/coda/pioctl.c      |    2 +-
 fs/hfs/inode.c        |    2 +-
 fs/namei.c            |   38 +++++++++++++++++++++++---------------
 fs/nfs/dir.c          |    3 +++
 fs/proc/proc_sysctl.c |    3 +++
 fs/smbfs/file.c       |    6 +++---
 include/linux/fs.h    |    1 +
 9 files changed, 40 insertions(+), 21 deletions(-)

Index: linux-2.6/fs/cifs/cifsfs.c
===================================================================
--- linux-2.6.orig/fs/cifs/cifsfs.c	2008-05-21 13:41:30.000000000 +0200
+++ linux-2.6/fs/cifs/cifsfs.c	2008-05-21 13:41:36.000000000 +0200
@@ -276,7 +276,7 @@ static int cifs_permission(struct dentry
 	cifs_sb = CIFS_SB(inode->i_sb);
 
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_PERM)
-		return 0;
+		return exec_permission(inode, mask);
 	else /* file mode might have been restricted at mount time
 		on the client (above and beyond ACL on servers) for
 		servers which do not support setting and viewing mode bits,
Index: ...
[ message continues ]

Hmm... What if !(mask & MAY_EXEC)? AFAICS, the above will return 0.
A better approach would be to use something like

	if (!(mask & MAY_EXEC))
		return -EACCES;
	if (S_ISREG(inode->i_mode) && !(inode->i_mode & S_IXUGO))
		return -EACCES;
	return 0;



--

[ message continues ]

No, we don't want to deny read or write (that's up to the filesystem
how it handles it), just want to deny execute if no x bits are set in
the mode.

I didn't really pay attention to individual filesystems, including
NFS, just mechanically moved the check from permission() to
->permission().  So it's possible that the code could be further
optimized in some cases.

Miklos
--

[ message continues ]

OK, but when I see something with the name 'exec_permission()', I assume
that it is going to check for whether or not I have execute permission.

If that is not the case, then can we please either change the function,
or change the name?

Cheers
  Trond

--

[ message continues ]

Yeah, the name's not very good.  Especially that there's an
exec_permission_lite() in the same file, which has nothing to do with
this.

Will try to think of something better.

Miklos
--

[ message continues ]

There's no point moving this into the filesystem.

--

[ message continues ]

Yes, there is.  Fuse already has special code, because it need to
refresh the attributes first before doing the "are there any x bits in
mode?" check.  And because there's some duplication, I've introduced a
helper.  That's exactly how we handle this sort of thing.

Miklos
--

[ message continues ]