2014-05-23 11:58:14

by Kinglong Mee

Subject: [PATCH 1/4] NFS4: Avoid NULL reference or double free in nfsd4_fslocs_free()

If fsloc_parse() failed at kzalloc(), fs/nfsd/export.c
411
412 fsloc->locations = kzalloc(fsloc->locations_count
413 * sizeof(struct nfsd4_fs_location), GFP_KERNEL);
414 if (!fsloc->locations)
415 return -ENOMEM;

svc_export_parse() will call nfsd4_fslocs_free() with fsloc->locations = NULL,
so that, "kfree(fsloc->locations[i].path);" will cause a crash.

If fsloc_parse() failed after that, fsloc_parse() will call nfsd4_fslocs_free(),
and svc_export_parse() will call it again, so that, a double free is caused.

This patch checks the fsloc->locations, and set to NULL after it be freed.

Signed-off-by: Kinglong Mee <[email protected]>
---
fs/nfsd/export.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 8513c59..9a41d3d 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -295,13 +295,19 @@ svc_expkey_update(struct cache_detail *cd, struct svc_expkey *new,

static void nfsd4_fslocs_free(struct nfsd4_fs_locations *fsloc)
{
+ struct nfsd4_fs_location *locations = fsloc->locations;
int i;

+ if (!locations)
+ return;
+
for (i = 0; i < fsloc->locations_count; i++) {
- kfree(fsloc->locations[i].path);
- kfree(fsloc->locations[i].hosts);
+ kfree(locations[i].path);
+ kfree(locations[i].hosts);
}
- kfree(fsloc->locations);
+
+ kfree(locations);
+ fsloc->locations = NULL;
}

static void svc_export_put(struct kref *ref)
--
1.9.0

2014-05-23 13:58:10

by J. Bruce Fields

Subject: Re: [PATCH 1/4] NFS4: Avoid NULL reference or double free in nfsd4_fslocs_free()

On Fri, May 23, 2014 at 07:57:49PM +0800, Kinglong Mee wrote:
> If fsloc_parse() failed at kzalloc(), fs/nfsd/export.c
> 411
> 412 fsloc->locations = kzalloc(fsloc->locations_count
> 413 * sizeof(struct nfsd4_fs_location), GFP_KERNEL);
> 414 if (!fsloc->locations)
> 415 return -ENOMEM;
>
> svc_export_parse() will call nfsd4_fslocs_free() with fsloc->locations = NULL,
> so that, "kfree(fsloc->locations[i].path);" will cause a crash.
>
> If fsloc_parse() failed after that, fsloc_parse() will call nfsd4_fslocs_free(),
> and svc_export_parse() will call it again, so that, a double free is caused.
>
> This patch checks the fsloc->locations, and set to NULL after it be freed.

Applying this and 2/4.

--b.

>
> Signed-off-by: Kinglong Mee <[email protected]>
> ---
> fs/nfsd/export.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 8513c59..9a41d3d 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -295,13 +295,19 @@ svc_expkey_update(struct cache_detail *cd, struct svc_expkey *new,
>
> static void nfsd4_fslocs_free(struct nfsd4_fs_locations *fsloc)
> {
> + struct nfsd4_fs_location *locations = fsloc->locations;
> int i;
>
> + if (!locations)
> + return;
> +
> for (i = 0; i < fsloc->locations_count; i++) {
> - kfree(fsloc->locations[i].path);
> - kfree(fsloc->locations[i].hosts);
> + kfree(locations[i].path);
> + kfree(locations[i].hosts);
> }
> - kfree(fsloc->locations);
> +
> + kfree(locations);
> + fsloc->locations = NULL;
> }
>
> static void svc_export_put(struct kref *ref)
> --
> 1.9.0
>