On 10/11/2022 2:46 AM, Ronnie Sahlberg wrote:
When application has done lseek() to a different offset on a directory fd
we skipped one entry too many before we start emitting directory entries
from the cache.
We need to also make sure that when we are starting to emit directory
entries from the cache, the ->pos sequence might have holes and skip
some indices.
Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
---
fs/cifs/readdir.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 8e060c00c969..7170614434a1 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -847,14 +847,19 @@ static bool emit_cached_dirents(struct cached_dirents *cde,
int rc;
list_for_each_entry(dirent, &cde->entries, entry) {
- if (ctx->pos >= dirent->pos)
+ if (ctx->pos > dirent->pos)
continue;
+ /*
+ * There may be holes in the ->pos sequence
+ * so always force ctx->pos to the current position.
+ */
The comment is a bit vague by referring to "->pos", because
it's the same name in both ctx and dirent.
But I have a second question, does squeezing out the holes
affect a later possible lseek on the dirhandle? I'm having
trouble tracking down where that might happen.
ctx->pos = dirent->pos;
rc = dir_emit(ctx, dirent->name, dirent->namelen,
dirent->fattr.cf_uniqueid,
dirent->fattr.cf_dtype);
if (!rc)
return rc;
It's weird that "rc" is an integer, but dir_emit() returns a bool.
It's also confusing that this isn't coded as
if (!rc)
return false;
Other than those questions, it looks like a clever fix.
Tom.
+ ctx->pos++;
}
return true;
}
@@ -1202,10 +1207,10 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
ctx->pos, tmp_buf);
cifs_save_resume_key(current_entry, cifsFile);
break;
- } else
- current_entry =
- nxt_dir_entry(current_entry, end_of_smb,
- cifsFile->srch_inf.info_level);
+ }
+ current_entry =
+ nxt_dir_entry(current_entry, end_of_smb,
+ cifsFile->srch_inf.info_level);
}
kfree(tmp_buf);
