Discussion:
[PATCH 0/3] Bugfixes for v3.17-rc
Pavel Shilovsky
2014-08-22 09:32:08 UTC
Permalink
Here is the set of bugfixes that was found during XFS testing (as a part of cifs-tests).
The patches are targeted for v3.17-rc and for stable as well.

Pavel Shilovsky (3):
CIFS: Fix directory rename error
CIFS: Fix wrong restart readdir for SMB1
CIFS: Fix wrong filename length for SMB2

fs/cifs/cifsglob.h | 5 -----
fs/cifs/inode.c | 5 ++++-
fs/cifs/readdir.c | 2 +-
fs/cifs/smb2file.c | 2 +-
fs/cifs/smb2inode.c | 2 +-
fs/cifs/smb2ops.c | 2 +-
fs/cifs/smb2pdu.c | 2 +-
7 files changed, 9 insertions(+), 11 deletions(-)
--
1.9.1
Pavel Shilovsky
2014-08-22 09:32:09 UTC
Permalink
CIFS servers process nlink counts differently for files and directories.
In cifs_rename() if we the request fails on the existing target, we
try to remove it through cifs_unlink() but this is not what we want
to do for directories. As the result the following sequence of commands

mkdir {1,2}; mv -T 1 2; rmdir {1,2}; mkdir {1,2}; echo foo > 2/bar

and XFS test generic/023 fail with -ENOENT error. That's why the second
mkdir reuses the existing inode (target inode of the mv -T command) with
S_DEAD flag.

Fix this by checking whether the target is directory or not and
calling cifs_rmdir() rather than cifs_unlink() for directories.

Cc: <stable-***@public.gmane.org>
Signed-off-by: Pavel Shilovsky <pshilovsky-***@public.gmane.org>
---
fs/cifs/inode.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 949ec90..7899a40 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1720,7 +1720,10 @@ cifs_rename2(struct inode *source_dir, struct dentry *source_dentry,
unlink_target:
/* Try unlinking the target dentry if it's not negative */
if (target_dentry->d_inode && (rc == -EACCES || rc == -EEXIST)) {
- tmprc = cifs_unlink(target_dir, target_dentry);
+ if (d_is_dir(target_dentry))
+ tmprc = cifs_rmdir(target_dir, target_dentry);
+ else
+ tmprc = cifs_unlink(target_dir, target_dentry);
if (tmprc)
goto cifs_rename_exit;
rc = cifs_do_rename(xid, source_dentry, from_name,
--
1.9.1
Pavel Shilovsky
2014-08-22 09:32:10 UTC
Permalink
The existing code calls server->ops->close() that is not
right. This causes XFS test generic/310 to fail. Fix this
by using server->ops->closedir() function.

Cc: <stable-***@public.gmane.org> # v3.7+
Signed-off-by: Pavel Shilovsky <pshilovsky-***@public.gmane.org>
---
fs/cifs/readdir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 798c80a..58e413c 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -597,7 +597,7 @@ find_cifs_entry(const unsigned int xid, struct cifs_tcon *tcon, loff_t pos,
cfile->invalidHandle = true;
spin_unlock(&cifs_file_list_lock);
if (server->ops->close)
- server->ops->close(xid, tcon, &cfile->fid);
+ server->ops->close_dir(xid, tcon, &cfile->fid);
} else
spin_unlock(&cifs_file_list_lock);
if (cfile->srch_inf.ntwrk_buf_start) {
--
1.9.1
Pavel Shilovsky
2014-08-22 09:32:11 UTC
Permalink
The existing code uses the old MAX_NAME constant. This causes
XFS test generic/013 to fail. Fix it by replacing MAX_NAME with
PATH_MAX that SMB1 uses. Also remove an unused MAX_NAME constant
definition.

Cc: <***@vger.kernel.org> # v3.7+
Signed-off-by: Pavel Shilovsky <***@samba.org>
---
fs/cifs/cifsglob.h | 5 -----
fs/cifs/smb2file.c | 2 +-
fs/cifs/smb2inode.c | 2 +-
fs/cifs/smb2ops.c | 2 +-
fs/cifs/smb2pdu.c | 2 +-
5 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 31f8ecc..636e1f9 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -70,11 +70,6 @@
#define SERVER_NAME_LENGTH 40
#define SERVER_NAME_LEN_WITH_NULL (SERVER_NAME_LENGTH + 1)

-/* used to define string lengths for reversing unicode strings */
-/* (256+1)*2 = 514 */
-/* (max path length + 1 for null) * 2 for unicode */
-#define MAX_NAME 514
-
/* SMB echo "timeout" -- FIXME: tunable? */
#define SMB_ECHO_INTERVAL (60 * HZ)

diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
index 3f17b45..4599294 100644
--- a/fs/cifs/smb2file.c
+++ b/fs/cifs/smb2file.c
@@ -50,7 +50,7 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
goto out;
}

- smb2_data = kzalloc(sizeof(struct smb2_file_all_info) + MAX_NAME * 2,
+ smb2_data = kzalloc(sizeof(struct smb2_file_all_info) + PATH_MAX * 2,
GFP_KERNEL);
if (smb2_data == NULL) {
rc = -ENOMEM;
diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index 0150182..899bbc8 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -131,7 +131,7 @@ smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon,
*adjust_tz = false;
*symlink = false;

- smb2_data = kzalloc(sizeof(struct smb2_file_all_info) + MAX_NAME * 2,
+ smb2_data = kzalloc(sizeof(struct smb2_file_all_info) + PATH_MAX * 2,
GFP_KERNEL);
if (smb2_data == NULL)
return -ENOMEM;
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 4e4eecd..f522193 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -389,7 +389,7 @@ smb2_query_file_info(const unsigned int xid, struct cifs_tcon *tcon,
int rc;
struct smb2_file_all_info *smb2_data;

- smb2_data = kzalloc(sizeof(struct smb2_file_all_info) + MAX_NAME * 2,
+ smb2_data = kzalloc(sizeof(struct smb2_file_all_info) + PATH_MAX * 2,
GFP_KERNEL);
if (smb2_data == NULL)
return -ENOMEM;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 0709494..28e5a12 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1533,7 +1533,7 @@ SMB2_query_info(const unsigned int xid, struct cifs_tcon *tcon,
{
return query_info(xid, tcon, persistent_fid, volatile_fid,
FILE_ALL_INFORMATION,
- sizeof(struct smb2_file_all_info) + MAX_NAME * 2,
+ sizeof(struct smb2_file_all_info) + PATH_MAX * 2,
sizeof(struct smb2_file_all_info), data);
}
--
1.9.1
Steve French
2014-08-22 16:08:39 UTC
Permalink
Merged into cifs-2.6.git for-next

This is fantastic that xfstests have already been so useful. This
will help a lot.

And I also saw that the cthon test improvements were merged into Steve
D's tree. We need to update the (Samba? and xfstest?) wikis with
information on how to run these.
Post by Pavel Shilovsky
Here is the set of bugfixes that was found during XFS testing (as a part of cifs-tests).
The patches are targeted for v3.17-rc and for stable as well.
CIFS: Fix directory rename error
CIFS: Fix wrong restart readdir for SMB1
CIFS: Fix wrong filename length for SMB2
fs/cifs/cifsglob.h | 5 -----
fs/cifs/inode.c | 5 ++++-
fs/cifs/readdir.c | 2 +-
fs/cifs/smb2file.c | 2 +-
fs/cifs/smb2inode.c | 2 +-
fs/cifs/smb2ops.c | 2 +-
fs/cifs/smb2pdu.c | 2 +-
7 files changed, 9 insertions(+), 11 deletions(-)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Thanks,

Steve
Loading...