Re: [PATCH-V2] [fs/9P] Add posixacl mount option

["Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>]

On Wed, 26 Jan 2011 22:55:28 -0800, "Venkateswararao Jujjuri (JV)" <jvrao@linux.vnet.ibm.com> wrote:
> The mount option access=client is overloaded as it assumes acl too.
> Adding posixacl option to enable POSIX ACLs makes it explicit and clear.
> Also it is convenient in the future to add other types of acls like richacls.
> 
> Ideally, the access mode 'client' should be just like V9FS_ACCESS_USER
> except it underscores the location of access check.
> Traditional 9P protocol lets the server perform access checks but with
> this mode, all the access checks will be performed on the client itself.
> Server just follows the client's directive.
> 
> Changes from V1
> --------------
> - Changed the option to -o posixacl and corresponding changes.
> - Corrected if conditions in acl.c
> 
> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
>  fs/9p/acl.c       |    8 +++++---
>  fs/9p/v9fs.c      |   24 +++++++++++++++++-------
>  fs/9p/v9fs.h      |    6 +++++-
>  fs/9p/vfs_super.c |    2 +-
>  4 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/9p/acl.c b/fs/9p/acl.c
> index 0a2e480..1ee3434 100644
> --- a/fs/9p/acl.c
> +++ b/fs/9p/acl.c
> @@ -59,7 +59,8 @@ int v9fs_get_acl(struct inode *inode, struct p9_fid *fid)
>  	struct v9fs_session_info *v9ses;
> 
>  	v9ses = v9fs_inode2v9ses(inode);
> -	if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) {
> +	if (((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) ||
> +			((v9ses->flags & V9FS_ACL_MASK) != V9FS_POSIX_ACL)) {
>  		set_cached_acl(inode, ACL_TYPE_DEFAULT, NULL);
>  		set_cached_acl(inode, ACL_TYPE_ACCESS, NULL);
>  		return 0;
> @@ -104,9 +105,10 @@ int v9fs_check_acl(struct inode *inode, int mask, unsigned int flags)
>  		return -ECHILD;
> 
>  	v9ses = v9fs_inode2v9ses(inode);
> -	if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) {
> +	if (((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) ||
> +			((v9ses->flags & V9FS_ACL_MASK) !=
> V9FS_POSIX_ACL)) {

check_acl should be called only if IS_POSIXACL(inode) is true. So do we
really need this check ?. Even the original change is not needed i guess.

>  		/*
> -		 * On access = client mode get the acl
> +		 * On access = client  and acl = on mode get the acl
>  		 * values from the server
>  		 */
>  		return 0;
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index d34f293..7823a7c 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -55,7 +55,7 @@ enum {
>  	/* Cache options */
>  	Opt_cache_loose, Opt_fscache,
>  	/* Access options */
> -	Opt_access,
> +	Opt_access, Opt_posixacl,
>  	/* Error token */
>  	Opt_err
>  };
> @@ -73,6 +73,7 @@ static const match_table_t tokens = {
>  	{Opt_fscache, "fscache"},
>  	{Opt_cachetag, "cachetag=%s"},
>  	{Opt_access, "access=%s"},
> +	{Opt_posixacl, "posixacl"},
>  	{Opt_err, NULL}
>  };
> 
> @@ -194,13 +195,7 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
>  			else if (strcmp(s, "any") == 0)
>  				v9ses->flags |= V9FS_ACCESS_ANY;
>  			else if (strcmp(s, "client") == 0) {
> -#ifdef CONFIG_9P_FS_POSIX_ACL
>  				v9ses->flags |= V9FS_ACCESS_CLIENT;
> -#else
> -				P9_DPRINTK(P9_DEBUG_ERROR,
> -					"Not defined CONFIG_9P_FS_POSIX_ACL. "
> -					"Ignoring access=client option\n");
> -#endif
>  			} else {
>  				v9ses->flags |= V9FS_ACCESS_SINGLE;
>  				v9ses->uid = simple_strtoul(s, &e, 10);
> @@ -210,6 +205,16 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
>  			kfree(s);
>  			break;
> 
> +		case Opt_posixacl:
> +#ifdef CONFIG_9P_FS_POSIX_ACL
> +			v9ses->flags |= V9FS_POSIX_ACL;
> +#else
> +			P9_DPRINTK(P9_DEBUG_ERROR,
> +					"Not defined CONFIG_9P_FS_POSIX_ACL. "
> +					"Ignoring posixacl option\n");
> +#endif

We should also ensure that -o posixacl is only allowed with
access=client. 

> +			break;
> +
>  		default:
>  			continue;
>  		}
> @@ -294,6 +299,11 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
>  		 */
>  		v9ses->flags &= ~V9FS_ACCESS_MASK;
>  		v9ses->flags |= V9FS_ACCESS_USER;
> +		/*
> +		 * We support ACL checks on clinet only if the protocol is
> +		 * 9P2000.L and access is V9FS_ACCESS_CLIENT.
> +		 */
> +		v9ses->flags &= ~V9FS_ACL_MASK;
>  	}

That is this need  update, 
access=user -o posixacl with dotu should not be allowed. 


>  	/*FIXME !! */
>  	/* for legacy mode, fall back to V9FS_ACCESS_ANY */
> diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
> index c4b5d88..b776a2c 100644
> --- a/fs/9p/v9fs.h
> +++ b/fs/9p/v9fs.h
> @@ -28,8 +28,10 @@
>   * @V9FS_PROTO_2000L: whether or not to use 9P2000.l extensions
>   * @V9FS_ACCESS_SINGLE: only the mounting user can access the hierarchy
>   * @V9FS_ACCESS_USER: a new attach will be issued for every user (default)
> + * @V9FS_ACCESS_CLIENT: Just like user, but access check is performed on client.
>   * @V9FS_ACCESS_ANY: use a single attach for all users
>   * @V9FS_ACCESS_MASK: bit mask of different ACCESS options
> + * @V9FS_ACL_ON: If ACLs are enforced

V9FS_POSIX_ACL ^^ that should be


>   *
>   * Session flags reflect options selected by users at mount time
>   */
> @@ -37,13 +39,15 @@
>  			 V9FS_ACCESS_USER |   \
>  			 V9FS_ACCESS_CLIENT)
>  #define V9FS_ACCESS_MASK V9FS_ACCESS_ANY
> +#define V9FS_ACL_MASK V9FS_POSIX_ACL
> 
>  enum p9_session_flags {
>  	V9FS_PROTO_2000U	= 0x01,
>  	V9FS_PROTO_2000L	= 0x02,
>  	V9FS_ACCESS_SINGLE	= 0x04,
>  	V9FS_ACCESS_USER	= 0x08,
> -	V9FS_ACCESS_CLIENT	= 0x10
> +	V9FS_ACCESS_CLIENT	= 0x10,
> +	V9FS_POSIX_ACL		= 0x20
>  };
> 
>  /* possible values of ->cache */
> diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
> index dbaabe3..4f14be5 100644
> --- a/fs/9p/vfs_super.c
> +++ b/fs/9p/vfs_super.c
> @@ -91,7 +91,7 @@ v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
>  	    MS_NOATIME;
> 
>  #ifdef CONFIG_9P_FS_POSIX_ACL
> -	if ((v9ses->flags & V9FS_ACCESS_MASK) == V9FS_ACCESS_CLIENT)
> +	if ((v9ses->flags & V9FS_ACL_MASK) == V9FS_POSIX_ACL)
>  		sb->s_flags |= MS_POSIXACL;
>  #endif
> 
> -- 
> 1.6.5.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html