Re: [Cluster-devel] [PATCH 1/8] cifs: Fix xattr name checks

2016-04-14 Thread Steve French
On Wed, Apr 13, 2016 at 11:59 PM, Al Viro  wrote:
> On Wed, Apr 13, 2016 at 11:43:19PM -0500, Steve French wrote:
>> If you are planning to merge it in next few weeks (they fix bugs, and
>> are safe so why not),  I can simply just back out my changes from
>> cifs-2.6.git for-next branch and let you merge the trivial checkpatch
>> cleanup attached.
>
> Do you want it in mainline before 4.6-final?  If that can wait until 4.7-rc1
> (and these are both old and not earth-shattering), I've no problem with
> putting the entire series (including cleanup) into a branch merged into
> vfs.git#for-next; if you consider it 4.6-final fodder, it's probably better
> be your pull request.

They are old, but I don't mind taking care of them now, and seem
low risk.

> PS: are there any docs re setup for cifs testing?
There are probably more but I have been using the page

https://wiki.samba.org/index.php/Xfstesting-cifs

cifs has more xfstest issues than NFS, but some of
the same kinds of problems (caching of metadata
time stamps e.g.) are similar.

-- 
Thanks,

Steve



Re: [Cluster-devel] [PATCH 1/8] cifs: Fix xattr name checks

2016-04-13 Thread Al Viro
On Wed, Apr 13, 2016 at 11:43:19PM -0500, Steve French wrote:
> If you are planning to merge it in next few weeks (they fix bugs, and
> are safe so why not),  I can simply just back out my changes from
> cifs-2.6.git for-next branch and let you merge the trivial checkpatch
> cleanup attached.

Do you want it in mainline before 4.6-final?  If that can wait until 4.7-rc1
(and these are both old and not earth-shattering), I've no problem with
putting the entire series (including cleanup) into a branch merged into
vfs.git#for-next; if you consider it 4.6-final fodder, it's probably better
be your pull request.

PS: are there any docs re setup for cifs testing?



Re: [Cluster-devel] [PATCH 1/8] cifs: Fix xattr name checks

2016-04-13 Thread Steve French
If you prefer merging those out of your tree, you can add my
Reviewed-by if needed (for the three cifs patches)

On Wed, Apr 13, 2016 at 11:43 PM, Steve French  wrote:
> If you are planning to merge it in next few weeks (they fix bugs, and
> are safe so why not),  I can simply just back out my changes from
> cifs-2.6.git for-next branch and let you merge the trivial checkpatch
> cleanup attached.
>
> On Wed, Apr 13, 2016 at 11:37 PM, Al Viro  wrote:
>> On Wed, Apr 13, 2016 at 11:26:24PM -0500, Steve French wrote:
>>> Thanks for spotting this - merged into cifs-2.6.git.   checkpatch
>>> spotted and old indentation issue so I cleaned that up in a followon
>>> patch that I will send.
>>
>> *ugh*
>>
>> And in the meanwhile I'd picked those into my queue...  Could you put that
>> stuff on a separate branch so that both cifs-2.6.git and vfs.git could
>> merge it?
>
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve



Re: [Cluster-devel] [PATCH 1/8] cifs: Fix xattr name checks

2016-04-13 Thread Steve French
If you are planning to merge it in next few weeks (they fix bugs, and
are safe so why not),  I can simply just back out my changes from
cifs-2.6.git for-next branch and let you merge the trivial checkpatch
cleanup attached.

On Wed, Apr 13, 2016 at 11:37 PM, Al Viro  wrote:
> On Wed, Apr 13, 2016 at 11:26:24PM -0500, Steve French wrote:
>> Thanks for spotting this - merged into cifs-2.6.git.   checkpatch
>> spotted and old indentation issue so I cleaned that up in a followon
>> patch that I will send.
>
> *ugh*
>
> And in the meanwhile I'd picked those into my queue...  Could you put that
> stuff on a separate branch so that both cifs-2.6.git and vfs.git could
> merge it?



-- 
Thanks,

Steve


0001-CIFS-Fix-formatting-issues-on-previous-patches-spott.patch
Description: application/mbox


Re: [Cluster-devel] [PATCH 1/8] cifs: Fix xattr name checks

2016-04-13 Thread Al Viro
On Wed, Apr 13, 2016 at 11:26:24PM -0500, Steve French wrote:
> Thanks for spotting this - merged into cifs-2.6.git.   checkpatch
> spotted and old indentation issue so I cleaned that up in a followon
> patch that I will send.

*ugh*

And in the meanwhile I'd picked those into my queue...  Could you put that
stuff on a separate branch so that both cifs-2.6.git and vfs.git could
merge it?



Re: [Cluster-devel] [PATCH 1/8] cifs: Fix xattr name checks

2016-04-13 Thread Steve French
Thanks for spotting this - merged into cifs-2.6.git.   checkpatch
spotted and old indentation issue so I cleaned that up in a followon
patch that I will send.

On Wed, Apr 13, 2016 at 5:30 PM, Andreas Gruenbacher
 wrote:
> Use strcmp(str, name) instead of strncmp(str, name, strlen(name)) for
> checking if str and name are the same (as opposed to name being a prefix
> of str) in the gexattr and setxattr inode operations.
>
> Signed-off-by: Andreas Gruenbacher 
> ---
>  fs/cifs/xattr.c | 24 
>  1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
> index 5d57c85..6e73ba9 100644
> --- a/fs/cifs/xattr.c
> +++ b/fs/cifs/xattr.c
> @@ -129,7 +129,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
> *ea_name,
>== 0) {
> if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
> goto set_ea_exit;
> -   if (strncmp(ea_name, CIFS_XATTR_DOS_ATTRIB, 14) == 0)
> +   if (strcmp(ea_name, CIFS_XATTR_DOS_ATTRIB) == 0)
> cifs_dbg(FYI, "attempt to set cifs inode metadata\n");
>
> ea_name += XATTR_USER_PREFIX_LEN; /* skip past user. prefix */
> @@ -147,8 +147,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
> *ea_name,
> rc = pTcon->ses->server->ops->set_EA(xid, pTcon,
> full_path, ea_name, ea_value, 
> (__u16)value_size,
> cifs_sb->local_nls, cifs_remap(cifs_sb));
> -   } else if (strncmp(ea_name, CIFS_XATTR_CIFS_ACL,
> -   strlen(CIFS_XATTR_CIFS_ACL)) == 0) {
> +   } else if (strcmp(ea_name, CIFS_XATTR_CIFS_ACL) == 0) {
>  #ifdef CONFIG_CIFS_ACL
> struct cifs_ntsd *pacl;
> pacl = kmalloc(value_size, GFP_KERNEL);
> @@ -170,10 +169,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
> *ea_name,
> cifs_dbg(FYI, "Set CIFS ACL not supported yet\n");
>  #endif /* CONFIG_CIFS_ACL */
> } else {
> -   int temp;
> -   temp = strncmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS,
> -   strlen(XATTR_NAME_POSIX_ACL_ACCESS));
> -   if (temp == 0) {
> +   if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS) == 0) {
>  #ifdef CONFIG_CIFS_POSIX
> if (sb->s_flags & MS_POSIXACL)
> rc = CIFSSMBSetPosixACL(xid, pTcon, full_path,
> @@ -184,8 +180,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
> *ea_name,
>  #else
> cifs_dbg(FYI, "set POSIX ACL not supported\n");
>  #endif
> -   } else if (strncmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT,
> -  strlen(XATTR_NAME_POSIX_ACL_DEFAULT)) == 
> 0) {
> +   } else if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT) == 
> 0) {
>  #ifdef CONFIG_CIFS_POSIX
> if (sb->s_flags & MS_POSIXACL)
> rc = CIFSSMBSetPosixACL(xid, pTcon, full_path,
> @@ -246,7 +241,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct 
> inode *inode,
> if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
> goto get_ea_exit;
>
> -   if (strncmp(ea_name, CIFS_XATTR_DOS_ATTRIB, 14) == 0) {
> +   if (strcmp(ea_name, CIFS_XATTR_DOS_ATTRIB) == 0) {
> cifs_dbg(FYI, "attempt to query cifs inode 
> metadata\n");
> /* revalidate/getattr then populate from inode */
> } /* BB add else when above is implemented */
> @@ -264,8 +259,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct 
> inode *inode,
> rc = pTcon->ses->server->ops->query_all_EAs(xid, 
> pTcon,
> full_path, ea_name, ea_value, buf_size,
> cifs_sb->local_nls, cifs_remap(cifs_sb));
> -   } else if (strncmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS,
> - strlen(XATTR_NAME_POSIX_ACL_ACCESS)) == 0) {
> +   } else if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS) == 0) {
>  #ifdef CONFIG_CIFS_POSIX
> if (sb->s_flags & MS_POSIXACL)
> rc = CIFSSMBGetPosixACL(xid, pTcon, full_path,
> @@ -275,8 +269,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct 
> inode *inode,
>  #else
> cifs_dbg(FYI, "Query POSIX ACL not supported yet\n");
>  #endif /* CONFIG_CIFS_POSIX */
> -   } else if (strncmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT,
> - strlen(XATTR_NAME_POSIX_ACL_DEFAULT)) == 0) {
> +   } else if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0) {
>  #ifdef CONFIG_CIFS_POSIX
> if (sb->s_flags & MS_POSIXACL)
> rc = CIFSSMBGetPosixACL(xid, pTcon, 

[Cluster-devel] [PATCH 1/8] cifs: Fix xattr name checks

2016-04-13 Thread Andreas Gruenbacher
Use strcmp(str, name) instead of strncmp(str, name, strlen(name)) for
checking if str and name are the same (as opposed to name being a prefix
of str) in the gexattr and setxattr inode operations.

Signed-off-by: Andreas Gruenbacher 
---
 fs/cifs/xattr.c | 24 
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
index 5d57c85..6e73ba9 100644
--- a/fs/cifs/xattr.c
+++ b/fs/cifs/xattr.c
@@ -129,7 +129,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
*ea_name,
   == 0) {
if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
goto set_ea_exit;
-   if (strncmp(ea_name, CIFS_XATTR_DOS_ATTRIB, 14) == 0)
+   if (strcmp(ea_name, CIFS_XATTR_DOS_ATTRIB) == 0)
cifs_dbg(FYI, "attempt to set cifs inode metadata\n");
 
ea_name += XATTR_USER_PREFIX_LEN; /* skip past user. prefix */
@@ -147,8 +147,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
*ea_name,
rc = pTcon->ses->server->ops->set_EA(xid, pTcon,
full_path, ea_name, ea_value, (__u16)value_size,
cifs_sb->local_nls, cifs_remap(cifs_sb));
-   } else if (strncmp(ea_name, CIFS_XATTR_CIFS_ACL,
-   strlen(CIFS_XATTR_CIFS_ACL)) == 0) {
+   } else if (strcmp(ea_name, CIFS_XATTR_CIFS_ACL) == 0) {
 #ifdef CONFIG_CIFS_ACL
struct cifs_ntsd *pacl;
pacl = kmalloc(value_size, GFP_KERNEL);
@@ -170,10 +169,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
*ea_name,
cifs_dbg(FYI, "Set CIFS ACL not supported yet\n");
 #endif /* CONFIG_CIFS_ACL */
} else {
-   int temp;
-   temp = strncmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS,
-   strlen(XATTR_NAME_POSIX_ACL_ACCESS));
-   if (temp == 0) {
+   if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS) == 0) {
 #ifdef CONFIG_CIFS_POSIX
if (sb->s_flags & MS_POSIXACL)
rc = CIFSSMBSetPosixACL(xid, pTcon, full_path,
@@ -184,8 +180,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
*ea_name,
 #else
cifs_dbg(FYI, "set POSIX ACL not supported\n");
 #endif
-   } else if (strncmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT,
-  strlen(XATTR_NAME_POSIX_ACL_DEFAULT)) == 0) {
+   } else if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0) {
 #ifdef CONFIG_CIFS_POSIX
if (sb->s_flags & MS_POSIXACL)
rc = CIFSSMBSetPosixACL(xid, pTcon, full_path,
@@ -246,7 +241,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct inode 
*inode,
if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
goto get_ea_exit;
 
-   if (strncmp(ea_name, CIFS_XATTR_DOS_ATTRIB, 14) == 0) {
+   if (strcmp(ea_name, CIFS_XATTR_DOS_ATTRIB) == 0) {
cifs_dbg(FYI, "attempt to query cifs inode metadata\n");
/* revalidate/getattr then populate from inode */
} /* BB add else when above is implemented */
@@ -264,8 +259,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct inode 
*inode,
rc = pTcon->ses->server->ops->query_all_EAs(xid, pTcon,
full_path, ea_name, ea_value, buf_size,
cifs_sb->local_nls, cifs_remap(cifs_sb));
-   } else if (strncmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS,
- strlen(XATTR_NAME_POSIX_ACL_ACCESS)) == 0) {
+   } else if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS) == 0) {
 #ifdef CONFIG_CIFS_POSIX
if (sb->s_flags & MS_POSIXACL)
rc = CIFSSMBGetPosixACL(xid, pTcon, full_path,
@@ -275,8 +269,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct inode 
*inode,
 #else
cifs_dbg(FYI, "Query POSIX ACL not supported yet\n");
 #endif /* CONFIG_CIFS_POSIX */
-   } else if (strncmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT,
- strlen(XATTR_NAME_POSIX_ACL_DEFAULT)) == 0) {
+   } else if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0) {
 #ifdef CONFIG_CIFS_POSIX
if (sb->s_flags & MS_POSIXACL)
rc = CIFSSMBGetPosixACL(xid, pTcon, full_path,
@@ -286,8 +279,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct inode 
*inode,
 #else
cifs_dbg(FYI, "Query POSIX default ACL not supported yet\n");
 #endif /* CONFIG_CIFS_POSIX */
-   } else if (strncmp(ea_name, CIFS_XATTR_CIFS_ACL,
-   strlen(CIFS_XATTR_CIFS_ACL)) == 0) {
+   } else if (strcmp(ea_name, CIFS_XATTR_CIFS_ACL) == 0) {
 #ifdef CONFIG_CIFS_ACL
   

[Cluster-devel] [PATCH 1/8] cifs: Fix xattr name checks

2016-04-13 Thread Andreas Gruenbacher
Use strcmp(str, name) instead of strncmp(str, name, strlen(name)) for
checking if str and name are the same (as opposed to name being a prefix
of str) in the gexattr and setxattr inode operations.

Signed-off-by: Andreas Gruenbacher 
---
 fs/cifs/xattr.c | 24 
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
index 5d57c85..6e73ba9 100644
--- a/fs/cifs/xattr.c
+++ b/fs/cifs/xattr.c
@@ -129,7 +129,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
*ea_name,
   == 0) {
if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
goto set_ea_exit;
-   if (strncmp(ea_name, CIFS_XATTR_DOS_ATTRIB, 14) == 0)
+   if (strcmp(ea_name, CIFS_XATTR_DOS_ATTRIB) == 0)
cifs_dbg(FYI, "attempt to set cifs inode metadata\n");
 
ea_name += XATTR_USER_PREFIX_LEN; /* skip past user. prefix */
@@ -147,8 +147,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
*ea_name,
rc = pTcon->ses->server->ops->set_EA(xid, pTcon,
full_path, ea_name, ea_value, (__u16)value_size,
cifs_sb->local_nls, cifs_remap(cifs_sb));
-   } else if (strncmp(ea_name, CIFS_XATTR_CIFS_ACL,
-   strlen(CIFS_XATTR_CIFS_ACL)) == 0) {
+   } else if (strcmp(ea_name, CIFS_XATTR_CIFS_ACL) == 0) {
 #ifdef CONFIG_CIFS_ACL
struct cifs_ntsd *pacl;
pacl = kmalloc(value_size, GFP_KERNEL);
@@ -170,10 +169,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
*ea_name,
cifs_dbg(FYI, "Set CIFS ACL not supported yet\n");
 #endif /* CONFIG_CIFS_ACL */
} else {
-   int temp;
-   temp = strncmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS,
-   strlen(XATTR_NAME_POSIX_ACL_ACCESS));
-   if (temp == 0) {
+   if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS) == 0) {
 #ifdef CONFIG_CIFS_POSIX
if (sb->s_flags & MS_POSIXACL)
rc = CIFSSMBSetPosixACL(xid, pTcon, full_path,
@@ -184,8 +180,7 @@ int cifs_setxattr(struct dentry *direntry, const char 
*ea_name,
 #else
cifs_dbg(FYI, "set POSIX ACL not supported\n");
 #endif
-   } else if (strncmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT,
-  strlen(XATTR_NAME_POSIX_ACL_DEFAULT)) == 0) {
+   } else if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0) {
 #ifdef CONFIG_CIFS_POSIX
if (sb->s_flags & MS_POSIXACL)
rc = CIFSSMBSetPosixACL(xid, pTcon, full_path,
@@ -246,7 +241,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct inode 
*inode,
if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
goto get_ea_exit;
 
-   if (strncmp(ea_name, CIFS_XATTR_DOS_ATTRIB, 14) == 0) {
+   if (strcmp(ea_name, CIFS_XATTR_DOS_ATTRIB) == 0) {
cifs_dbg(FYI, "attempt to query cifs inode metadata\n");
/* revalidate/getattr then populate from inode */
} /* BB add else when above is implemented */
@@ -264,8 +259,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct inode 
*inode,
rc = pTcon->ses->server->ops->query_all_EAs(xid, pTcon,
full_path, ea_name, ea_value, buf_size,
cifs_sb->local_nls, cifs_remap(cifs_sb));
-   } else if (strncmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS,
- strlen(XATTR_NAME_POSIX_ACL_ACCESS)) == 0) {
+   } else if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_ACCESS) == 0) {
 #ifdef CONFIG_CIFS_POSIX
if (sb->s_flags & MS_POSIXACL)
rc = CIFSSMBGetPosixACL(xid, pTcon, full_path,
@@ -275,8 +269,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct inode 
*inode,
 #else
cifs_dbg(FYI, "Query POSIX ACL not supported yet\n");
 #endif /* CONFIG_CIFS_POSIX */
-   } else if (strncmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT,
- strlen(XATTR_NAME_POSIX_ACL_DEFAULT)) == 0) {
+   } else if (strcmp(ea_name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0) {
 #ifdef CONFIG_CIFS_POSIX
if (sb->s_flags & MS_POSIXACL)
rc = CIFSSMBGetPosixACL(xid, pTcon, full_path,
@@ -286,8 +279,7 @@ ssize_t cifs_getxattr(struct dentry *direntry, struct inode 
*inode,
 #else
cifs_dbg(FYI, "Query POSIX default ACL not supported yet\n");
 #endif /* CONFIG_CIFS_POSIX */
-   } else if (strncmp(ea_name, CIFS_XATTR_CIFS_ACL,
-   strlen(CIFS_XATTR_CIFS_ACL)) == 0) {
+   } else if (strcmp(ea_name, CIFS_XATTR_CIFS_ACL) == 0) {
 #ifdef CONFIG_CIFS_ACL