On Sun, 10 Oct 2010 15:31:56 -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.
>
> Do get CIFS/NTFS ACL for xattr for attribute system.posix_acl_access
>
>
> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> ---
> fs/cifs/cifsacl.c | 2 +-
> fs/cifs/cifsproto.h | 2 ++
> fs/cifs/xattr.c | 41 ++++++++++++++++++++---------------------
> 3 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> index c9b4792..bafe774 100644
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -607,7 +607,7 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
> }
>
> /* Retrieve an ACL from the server */
> -static struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb,
> +struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb,
> struct inode *inode, const char *path,
> u32 *pacllen)
> {
^^^^^^^^^^^^
get_cifs_acl has "issues". This would be a good opportunity to fix them...
If cifs_get_acl_by_path fails, then it returns a NULL pointer. The
error code that might give some hint about why it failed is lost. It
ought to instead return a ERR_PTR() and have that bubble up to the
callers.
You should do a patch that fixes that first (and its callers, of
course), and then rebase this patch on that.
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 29a2ee8..229156e 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -134,6 +134,8 @@ extern void cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb,
> struct cifs_fattr *fattr, struct inode *inode,
> const char *path, const __u16 *pfid);
> extern int mode_to_acl(struct inode *inode, const char *path, __u64);
> +extern struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *, struct inode *,
> + const char *, u32 *);
>
> extern int cifs_mount(struct super_block *, struct cifs_sb_info *, char *,
> const char *);
> diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
> index a264b74..6899438 100644
> --- a/fs/cifs/xattr.c
> +++ b/fs/cifs/xattr.c
> @@ -30,6 +30,7 @@
>
> #define MAX_EA_VALUE_SIZE 65535
> #define CIFS_XATTR_DOS_ATTRIB "user.DosAttrib"
> +#define CIFS_ACL_XATTR "system.cifsacl"
^^^^^^^^^^
This should probably be defined in terms of CIFS_XATTR_SYSTEM_PREFIX.
Ditto for CIFS_XATTR_DOS_ATTRIB, even though that wasn't done by your
patch.
Also, NFSv4 uses "system.nfs4_acl". It would be nice to follow that
convention. Maybe rename this "system.cifs_acl" instead?
> #define CIFS_XATTR_USER_PREFIX "user."
> #define CIFS_XATTR_SYSTEM_PREFIX "system."
> #define CIFS_XATTR_OS2_PREFIX "os2."
> @@ -217,11 +218,13 @@ ssize_t cifs_getxattr(struct dentry *direntry, const char *ea_name,
> ssize_t rc = -EOPNOTSUPP;
> #ifdef CONFIG_CIFS_XATTR
> int xid;
> + u32 acllen;
> struct cifs_sb_info *cifs_sb;
> struct tcon_link *tlink;
> struct cifsTconInfo *pTcon;
> struct super_block *sb;
> char *full_path;
> + struct cifs_ntsd *pacl;
>
> if (direntry == NULL)
> return -EIO;
> @@ -277,27 +280,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 +295,23 @@ ssize_t cifs_getxattr(struct dentry *direntry, const char *ea_name,
> #else
> cFYI(1, "query POSIX default ACL not supported yet");
> #endif
> + } else if (strncmp(ea_name, CIFS_ACL_XATTR,
> + strlen(CIFS_ACL_XATTR)) == 0) {
> + pacl = get_cifs_acl(cifs_sb, direntry->d_inode,
> + full_path, &acllen);
> + if (!pacl) {
> + cERROR(1, "get CIFS ACL xattr error");
> + rc = -EIO;
> + } else {
> + if (ea_value) {
> + if (acllen > buf_size)
> + acllen = -ERANGE;
> + else
> + memcpy(ea_value, pacl, acllen);
> + }
> + rc = acllen;
> + kfree(pacl);
> + }
> } else if (strncmp(ea_name,
> CIFS_XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN) == 0) {
> cFYI(1, "Trusted xattr namespace not supported yet");
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
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
