Fix for Bug 13107 - two missing files if mount to Windows root (of drive) shares

Message ID CAH2r5mup08gA=KNPbaFbD=J8V6PRtqJH4Qtoh2d5vckjgtKs_g@mail.gmail.com
State New
Headers show
Series
  • Fix for Bug 13107 - two missing files if mount to Windows root (of drive) shares
Related show

Commit Message

Steve French April 12, 2018, 6:05 a.m.
Windows doesn't always return . and .. (e.g.) for when you mount to
equivalent of C$
this causes first two directory entries to be skipped.  Patch attached
which seems to fix it (more testing would be appreciated)

I suspect that there is a better way to fix this - anyone see a better
way.  Problem is that if we rely on ctx->pos to determine where we are
in the buffer we end up starting on entry 3 in the serach results ...
but entry 1 and 2 are not always . and .. (as we see in shares in
windows to the root of a drive).

Any better ideas?

Comments

Pavel Shilovsky April 12, 2018, 9:59 p.m. | #1
2018-04-11 23:05 GMT-07:00 Steve French <smfrench@gmail.com>:
> Windows doesn't always return . and .. (e.g.) for when you mount to
> equivalent of C$
> this causes first two directory entries to be skipped.  Patch attached
> which seems to fix it (more testing would be appreciated)
>
> I suspect that there is a better way to fix this - anyone see a better
> way.  Problem is that if we rely on ctx->pos to determine where we are
> in the buffer we end up starting on entry 3 in the serach results ...
> but entry 1 and 2 are not always . and .. (as we see in shares in
> windows to the root of a drive).
>
> Any better ideas?
>
>
>
> --
> Thanks,
>
> Steve



Can we make SM2_query_directory() and FindFirst() return a predictable
format of output by removing . and .. for the response buffer (if they
exist)? This will allow us to make cifs_readdir() cleaner.

--
Best regards,
Pavel Shilovsky
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve French April 12, 2018, 10:01 p.m. | #2
I agree that something like this would be cleaner - needs more testing
too ... am backing this fix out for time being.

On Thu, Apr 12, 2018 at 4:59 PM, Pavel Shilovsky <piastryyy@gmail.com> wrote:
> 2018-04-11 23:05 GMT-07:00 Steve French <smfrench@gmail.com>:
>> Windows doesn't always return . and .. (e.g.) for when you mount to
>> equivalent of C$
>> this causes first two directory entries to be skipped.  Patch attached
>> which seems to fix it (more testing would be appreciated)
>>
>> I suspect that there is a better way to fix this - anyone see a better
>> way.  Problem is that if we rely on ctx->pos to determine where we are
>> in the buffer we end up starting on entry 3 in the serach results ...
>> but entry 1 and 2 are not always . and .. (as we see in shares in
>> windows to the root of a drive).
>>
>> Any better ideas?
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
>
>
>
> Can we make SM2_query_directory() and FindFirst() return a predictable
> format of output by removing . and .. for the response buffer (if they
> exist)? This will allow us to make cifs_readdir() cleaner.
>
> --
> Best regards,
> Pavel Shilovsky

Patch

From 73101ab3ac801638efaa143f2b4d5a244c8d1fed Mon Sep 17 00:00:00 2001
From: Steve French <smfrench@gmail.com>
Date: Thu, 12 Apr 2018 00:52:49 -0500
Subject: [PATCH] SMB3: Fix missing files when server doesn't return . and ..

If the server doesn't return . or .. then the starting file
returned is wrong and the first two files from the
directory will be skipped.  You can see this on mounts to Windows
shares that are on the root of a drive for example.

Fixes https://bugzilla.samba.org/show_bug.cgi?id=13107

CC: Stable <stable@vger.kernel.org>
Signed-off-by: Steve French <smfrench@gmail.com>
---
 fs/cifs/readdir.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index a27fc8791551..3325cabe0850 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -779,6 +779,7 @@  int cifs_readdir(struct file *file, struct dir_context *ctx)
 	struct cifsFileInfo *cifsFile = NULL;
 	char *current_entry;
 	int num_to_fill = 0;
+	loff_t first_entry;
 	char *tmp_buf = NULL;
 	char *end_of_smb;
 	unsigned int max_len;
@@ -796,6 +797,7 @@  int cifs_readdir(struct file *file, struct dir_context *ctx)
 			goto rddir2_exit;
 	}
 
+	first_entry = ctx->pos;
 	if (!dir_emit_dots(file, ctx))
 		goto rddir2_exit;
 
@@ -817,7 +819,7 @@  int cifs_readdir(struct file *file, struct dir_context *ctx)
 	} */
 
 	tcon = tlink_tcon(cifsFile->tlink);
-	rc = find_cifs_entry(xid, tcon, ctx->pos, file, &current_entry,
+	rc = find_cifs_entry(xid, tcon, first_entry, file, &current_entry,
 			     &num_to_fill);
 	if (rc) {
 		cifs_dbg(FYI, "fce error %d\n", rc);
@@ -848,8 +850,8 @@  int cifs_readdir(struct file *file, struct dir_context *ctx)
 			break;
 		}
 		/*
-		 * if buggy server returns . and .. late do we want to
-		 * check for that here?
+		 * if buggy server returns . and .. late
+		 * it will be skipped in cifs_filldir
 		 */
 		*tmp_buf = 0;
 		rc = cifs_filldir(current_entry, file, ctx,
@@ -861,7 +863,7 @@  int cifs_readdir(struct file *file, struct dir_context *ctx)
 		}
 
 		ctx->pos++;
-		if (ctx->pos ==
+		if (ctx->pos == 2 +
 			cifsFile->srch_inf.index_of_last_entry) {
 			cifs_dbg(FYI, "last entry in buf at pos %lld %s\n",
 				 ctx->pos, tmp_buf);
-- 
2.14.1