| Message ID | 20200202225356.995080-2-trond.myklebust@hammerspace.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Readdir fixes | expand |
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
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; }
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> --- fs/nfs/dir.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)