linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


             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).