From: Trond Myklebust <trondmy@gmail.com>
To: linux-nfs@vger.kernel.org
Cc: Liguang Zhang <zhangliguang@linux.alibaba.com>
Subject: [PATCH 1/3] NFS: Fix off-by-one errors in nfs_readdir
Date: Sat, 6 Jul 2019 14:52:50 -0400 [thread overview]
Message-ID: <20190706185252.32488-1-trond.myklebust@hammerspace.com> (raw)
In C, the array size and the maximum index are not the same. In this
case, the field desc->pvec.nr is being used as a cursor but is
occasionally being treated as if it the array size.
This patch renames it to desc->pvec.cursor in order to make clear that
it is tracking an index.
Fixes: be4c2d4723a4 ("NFS: readdirplus optimization by cache mechanism")
Cc: Liguang Zhang <zhangliguang@linux.alibaba.com>
Cc: stable@vger.kernel.org # v5.1+
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/dir.c | 53 +++++++++++++++++++++++++---------------------------
1 file changed, 25 insertions(+), 28 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 57b6a45576ad..e44f3c9fad5b 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -134,14 +134,14 @@ struct nfs_cache_array_entry {
};
struct nfs_cache_array {
- int size;
- int eof_index;
u64 last_cookie;
+ unsigned int size;
+ bool eof;
struct nfs_cache_array_entry array[0];
};
struct readdirvec {
- unsigned long nr;
+ unsigned long cursor;
unsigned long index;
struct page *pages[NFS_MAX_READDIR_RAPAGES];
};
@@ -172,7 +172,7 @@ static
void nfs_readdir_clear_array(struct page *page)
{
struct nfs_cache_array *array;
- int i;
+ unsigned int i;
array = kmap_atomic(page);
for (i = 0; i < array->size; i++)
@@ -224,7 +224,7 @@ int nfs_readdir_add_to_array(struct nfs_entry *entry, struct page *page)
array->last_cookie = entry->cookie;
array->size++;
if (entry->eof != 0)
- array->eof_index = array->size;
+ array->eof = true;
out:
kunmap(page);
return ret;
@@ -239,7 +239,7 @@ int nfs_readdir_search_for_pos(struct nfs_cache_array *array, nfs_readdir_descri
if (diff < 0)
goto out_eof;
if (diff >= array->size) {
- if (array->eof_index >= 0)
+ if (array->eof)
goto out_eof;
return -EAGAIN;
}
@@ -265,7 +265,7 @@ nfs_readdir_inode_mapping_valid(struct nfs_inode *nfsi)
static
int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_descriptor_t *desc)
{
- int i;
+ unsigned int i;
loff_t new_pos;
int status = -EAGAIN;
@@ -300,7 +300,7 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_des
return 0;
}
}
- if (array->eof_index >= 0) {
+ if (array->eof) {
status = -EBADCOOKIE;
if (*desc->dir_cookie == array->last_cookie)
desc->eof = true;
@@ -532,10 +532,9 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
struct nfs_cache_array *array;
unsigned int count = 0;
int status;
- int max_rapages = NFS_MAX_READDIR_RAPAGES;
desc->pvec.index = desc->page_index;
- desc->pvec.nr = 0;
+ desc->pvec.cursor = 0;
scratch = alloc_page(GFP_KERNEL);
if (scratch == NULL)
@@ -560,12 +559,12 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
if (desc->plus)
nfs_prime_dcache(file_dentry(desc->file), entry);
- status = nfs_readdir_add_to_array(entry, desc->pvec.pages[desc->pvec.nr]);
+ status = nfs_readdir_add_to_array(entry, desc->pvec.pages[desc->pvec.cursor]);
if (status == -ENOSPC) {
- desc->pvec.nr++;
- if (desc->pvec.nr == max_rapages)
+ if (desc->pvec.cursor == ARRAY_SIZE(desc->pvec.pages) - 1)
break;
- status = nfs_readdir_add_to_array(entry, desc->pvec.pages[desc->pvec.nr]);
+ desc->pvec.cursor++;
+ status = nfs_readdir_add_to_array(entry, desc->pvec.pages[desc->pvec.cursor]);
}
if (status != 0)
break;
@@ -579,8 +578,8 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
out_nopages:
if (count == 0 || (status == -EBADCOOKIE && entry->eof != 0)) {
- array = kmap_atomic(desc->pvec.pages[desc->pvec.nr]);
- array->eof_index = array->size;
+ array = kmap_atomic(desc->pvec.pages[desc->pvec.cursor]);
+ array->eof = true;
status = 0;
kunmap_atomic(array);
}
@@ -588,11 +587,11 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
put_page(scratch);
/*
- * desc->pvec.nr > 0 means at least one page was completely filled,
+ * desc->pvec.cursor > 0 means at least one page was completely filled,
* we should return -ENOSPC. Otherwise function
* nfs_readdir_xdr_to_array will enter infinite loop.
*/
- if (desc->pvec.nr > 0)
+ if (desc->pvec.cursor > 0)
return -ENOSPC;
return status;
}
@@ -634,13 +633,12 @@ static
void nfs_readdir_rapages_init(nfs_readdir_descriptor_t *desc)
{
struct nfs_cache_array *array;
- int max_rapages = NFS_MAX_READDIR_RAPAGES;
- int index;
+ unsigned int max_rapages = NFS_MAX_READDIR_RAPAGES;
+ unsigned int index;
for (index = 0; index < max_rapages; index++) {
array = kmap_atomic(desc->pvec.pages[index]);
memset(array, 0, sizeof(struct nfs_cache_array));
- array->eof_index = -1;
kunmap_atomic(array);
}
}
@@ -678,7 +676,6 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
array = kmap(page);
memset(array, 0, sizeof(struct nfs_cache_array));
- array->eof_index = -1;
status = nfs_readdir_alloc_pages(pages, array_size);
if (status < 0)
@@ -696,7 +693,7 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
status = 0;
break;
}
- } while (array->eof_index < 0);
+ } while (!array->eof);
nfs_readdir_free_pages(pages, array_size);
out_release_array:
@@ -723,10 +720,10 @@ int nfs_readdir_filler(void *data, struct page* page)
/*
* If desc->page_index in range desc->pvec.index and
- * desc->pvec.index + desc->pvec.nr, we get readdir cache hit.
+ * desc->pvec.index + desc->pvec.cursor, we get readdir cache hit.
*/
if (desc->page_index >= desc->pvec.index &&
- desc->page_index < (desc->pvec.index + desc->pvec.nr)) {
+ desc->page_index <= (desc->pvec.index + desc->pvec.cursor)) {
/*
* page and desc->pvec.pages[x] are valid, don't need to check
* whether or not to be NULL.
@@ -809,7 +806,7 @@ static
int nfs_do_filldir(nfs_readdir_descriptor_t *desc)
{
struct file *file = desc->file;
- int i = 0;
+ unsigned int i = 0;
int res = 0;
struct nfs_cache_array *array = NULL;
struct nfs_open_dir_context *ctx = file->private_data;
@@ -832,7 +829,7 @@ int nfs_do_filldir(nfs_readdir_descriptor_t *desc)
if (ctx->duped != 0)
ctx->duped = 1;
}
- if (array->eof_index >= 0)
+ if (array->eof)
desc->eof = true;
kunmap(desc->page);
@@ -903,7 +900,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
*desc = &my_desc;
struct nfs_open_dir_context *dir_ctx = file->private_data;
int res = 0;
- int max_rapages = NFS_MAX_READDIR_RAPAGES;
+ unsigned int max_rapages = NFS_MAX_READDIR_RAPAGES;
dfprintk(FILE, "NFS: readdir(%pD2) starting at cookie %llu\n",
file, (long long)ctx->pos);
--
2.21.0
next reply other threads:[~2019-07-06 18:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-06 18:52 Trond Myklebust [this message]
2019-07-06 18:52 ` [PATCH 2/3] NFS: Reduce stack usage in nfs_readdir() Trond Myklebust
2019-07-06 18:52 ` [PATCH 3/3] NFS: Ensure cached readdir info is NUL terminated Trond Myklebust
2019-07-07 21:01 ` [PATCH 1/3] NFS: Fix off-by-one errors in nfs_readdir Mkrtchyan, Tigran
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190706185252.32488-1-trond.myklebust@hammerspace.com \
--to=trondmy@gmail.com \
--cc=linux-nfs@vger.kernel.org \
--cc=zhangliguang@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).