From: "Benjamin Coddington" <bcodding@redhat.com>
To: "Trond Myklebust" <trondmy@gmail.com>
Cc: "Anna Schumaker" <Anna.Schumaker@netapp.com>,
"Dai Ngo" <dai.ngo@oracle.com>,
linux-nfs@vger.kernel.org
Subject: Re: [PATCH 1/4] NFS: Fix memory leaks and corruption in readdir
Date: Mon, 03 Feb 2020 08:44:21 -0500 [thread overview]
Message-ID: <60888F6B-EA4B-4857-9B41-2F6087212D76@redhat.com> (raw)
In-Reply-To: <20200202225356.995080-2-trond.myklebust@hammerspace.com>
On 2 Feb 2020, at 17:53, Trond Myklebust wrote:
> nfs_readdir_xdr_to_array() must not exit without having initialised
> the array, so that the page cache deletion routines can safely
> call nfs_readdir_clear_array().
> Furthermore, we should ensure that if we exit nfs_readdir_filler()
> with an error, we free up any page contents to prevent a leak
> if we try to fill the page again.
>
> Fixes: 11de3b11e08c ("NFS: Fix a memory leak in nfs_readdir")
> Cc: stable@vger.kernel.org # v2.6.37+
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Looks good to me.
Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
Ben
> ---
> fs/nfs/dir.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 76404f53cf21..ba0d55930e8a 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -163,6 +163,17 @@ typedef struct {
> bool eof;
> } nfs_readdir_descriptor_t;
>
> +static
> +void nfs_readdir_init_array(struct page *page)
> +{
> + struct nfs_cache_array *array;
> +
> + array = kmap_atomic(page);
> + memset(array, 0, sizeof(struct nfs_cache_array));
> + array->eof_index = -1;
> + kunmap_atomic(array);
> +}
> +
> /*
> * we are freeing strings created by nfs_add_to_readdir_array()
> */
> @@ -175,6 +186,7 @@ void nfs_readdir_clear_array(struct page *page)
> array = kmap_atomic(page);
> for (i = 0; i < array->size; i++)
> kfree(array->array[i].string.name);
> + array->size = 0;
> kunmap_atomic(array);
> }
>
> @@ -613,6 +625,8 @@ int
> nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page
> *page,
> int status = -ENOMEM;
> unsigned int array_size = ARRAY_SIZE(pages);
>
> + nfs_readdir_init_array(page);
> +
> entry.prev_cookie = 0;
> entry.cookie = desc->last_cookie;
> entry.eof = 0;
> @@ -629,8 +643,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)
> @@ -685,6 +697,7 @@ int nfs_readdir_filler(void *data, struct page*
> page)
> unlock_page(page);
> return 0;
> error:
> + nfs_readdir_clear_array(page);
> unlock_page(page);
> return ret;
> }
> --
> 2.24.1
prev parent reply other threads:[~2020-02-03 13:44 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-02 22:53 [PATCH 0/4] Readdir fixes Trond Myklebust
2020-02-02 22:53 ` [PATCH 1/4] NFS: Fix memory leaks and corruption in readdir Trond Myklebust
2020-02-02 22:53 ` [PATCH 2/4] NFS: Directory page cache pages need to be locked when read Trond Myklebust
2020-02-02 22:53 ` [PATCH 3/4] NFS: Use kmemdup_nul() in nfs_readdir_make_qstr() Trond Myklebust
2020-02-02 22:53 ` [PATCH 4/4] NFS: Switch readdir to using iterate_shared() Trond Myklebust
2020-02-03 14:41 ` Benjamin Coddington
2020-02-03 14:22 ` [PATCH 3/4] NFS: Use kmemdup_nul() in nfs_readdir_make_qstr() Benjamin Coddington
2020-02-03 14:21 ` [PATCH 2/4] NFS: Directory page cache pages need to be locked when read Benjamin Coddington
2020-02-03 20:31 ` Schumaker, Anna
2020-02-03 21:18 ` Trond Myklebust
2020-02-03 21:21 ` Schumaker, Anna
2020-02-03 22:45 ` Trond Myklebust
2020-02-03 22:50 ` Trond Myklebust
2020-02-03 22:55 ` Trond Myklebust
2020-02-05 0:44 ` Dai Ngo
2020-02-03 13:44 ` Benjamin Coddington [this message]
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=60888F6B-EA4B-4857-9B41-2F6087212D76@redhat.com \
--to=bcodding@redhat.com \
--cc=Anna.Schumaker@netapp.com \
--cc=dai.ngo@oracle.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trondmy@gmail.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).