On Sun, Oct 10, 2010 at 9:15 PM, Steve French <smfrench@xxxxxxxxx> wrote:
> On Sat, Oct 9, 2010 at 3:42 PM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
>> On Thu, 7 Oct 2010 09:03:27 -0500
>> shirishpargaonkar@xxxxxxxxx wrote:
>>
>>> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>>>
>>>
>>> Add extended attribute name system.cifsacl
>>>
>>> Get/generate cifs/ntfs acl blob and hand over to the invoker however
>>> it wants to parse/process it.
>>>
>>> Stop generating cifs/ntfs acl blob for xattr name system.posix_acl_access
>>>
>>>
>>> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>>> ---
>>> fs/cifs/xattr.c | 64 +++++++++++++++++++++++++++++++++++++------------------
>>> 1 files changed, 43 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
>>> --- a/fs/cifs/xattr.c
>>> +++ b/fs/cifs/xattr.c
> <snip>
>>> +static int
>>> +get_cifs_acl(int xid, struct cifsTconInfo *pTcon, char *full_path,
>>> + void *ea_value, size_t size, struct nls_table *nls_cp)
>>> +{
>>> + __u16 fid;
>>> + int rc;
>>> + int oplock = 0;
>>> + unsigned int acllen = 0;
>>> + struct cifs_ntsd *pacl = NULL;
>>> +
>>> + rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_OPEN, GENERIC_READ,
>>> + 0, &fid, &oplock, NULL, nls_cp, 0);
>>> + if (rc) {
>>> + cERROR(1, "%s: file open error: %d", __func__, rc);
>>> + return rc;
>>> + }
>>> +
>> ^^^^
>> Hmm possible oplock break. Wouldn't it be better to see if you already
>> have a readable filehandle before you do this?
>
> Yes - absolutely need to check for matching file handle
>
>>> + rc = CIFSSMBGetCIFSACL(xid, pTcon, fid, &pacl, &acllen);
>>> + CIFSSMBClose(xid, pTcon, fid);
>>> + if (rc) {
>>> + cERROR(1, "%s: get acl error: %d", __func__, rc);
>>> + return rc;
>>> + }
>>> +
>>> + if (ea_value) {
>>> + if (acllen > size) {
>>> + kfree(pacl);
>>> + return -ERANGE;
>>> + }
>>> + memcpy(ea_value, pacl, acllen);
>>> + kfree(pacl);
>>> + }
>>> +
>>> + return acllen;
>>> +}
>>> +
>>> int cifs_setxattr(struct dentry *direntry, const char *ea_name,
>>> const void *ea_value, size_t value_size, int flags)
>>> {
>>> @@ -277,27 +314,6 @@ ssize_t cifs_getxattr(struct dentry *direntry, const char *ea_name,
>>> cifs_sb->local_nls,
>>> cifs_sb->mnt_cifs_flags &
>>> CIFS_MOUNT_MAP_SPECIAL_CHR);
>>> -#ifdef CONFIG_CIFS_EXPERIMENTAL
>>> - else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL) {
>>> - __u16 fid;
>>> - int oplock = 0;
>>> - struct cifs_ntsd *pacl = NULL;
>>> - __u32 buflen = 0;
>>> - if (experimEnabled)
>>> - rc = CIFSSMBOpen(xid, pTcon, full_path,
>>> - FILE_OPEN, GENERIC_READ, 0, &fid,
>>> - &oplock, NULL, cifs_sb->local_nls,
>>> - cifs_sb->mnt_cifs_flags &
>>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>>> - /* else rc is EOPNOTSUPP from above */
>>> -
>>> - if (rc == 0) {
>>> - rc = CIFSSMBGetCIFSACL(xid, pTcon, fid, &pacl,
>>> - &buflen);
>>> - CIFSSMBClose(xid, pTcon, fid);
>>> - }
>>> - }
>>> -#endif /* EXPERIMENTAL */
>>> #else
>>> cFYI(1, "query POSIX ACL not supported yet");
>>> #endif /* CONFIG_CIFS_POSIX */
>>> @@ -313,6 +329,12 @@ ssize_t cifs_getxattr(struct dentry *direntry, const char *ea_name,
>>> #else
>>> cFYI(1, "query POSIX default ACL not supported yet");
>>> #endif
>>> +#ifdef CONFIG_CIFS_EXPERIMENTAL
>>> + } else if (strncmp(ea_name, CIFS_ACL_XATTR,
>>> + strlen(CIFS_ACL_XATTR)) == 0) {
>>> + rc = get_cifs_acl(xid, pTcon, full_path, ea_value,
>>> + buf_size, cifs_sb->local_nls);
>>> +#endif /* CONFIG_CIFS_EXPERIMENTAL */
>>> } else if (strncmp(ea_name,
>>> CIFS_XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN) == 0) {
>>> cFYI(1, "Trusted xattr namespace not supported yet");
>>
>> I'd probably prefer not to see this new code wrapped in experimental
>> tags. I don't think that adds any real value, especially not with
>> something as simple as this.
>
> I agree - seems low risk as well.
>
> --
> Thanks,
>
> Steve
>
>
>
> --
> Thanks,
>
> Steve
>
Did resubmit the patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
