On 1/27/2011 6:40 AM, Aneesh Kumar K. V wrote:
> On Wed, 26 Jan 2011 22:55:28 -0800, "Venkateswararao Jujjuri (JV)" <jvrao@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
>> ---
>> 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.
May be not; if we need it to be deleted let us have another patch.
>
>> /*
>> - * 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.
Hrm.. that would make sense.. I can add a check here.
>
>
>> /*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
Good catch.
Thanks,
JV
>
>
>> *
>> * 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@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
