From: Cyrill Gorcunov <[email protected]>
There is possible NULL pointer dereference if kstr[n]dup failed.
So fix them for safety.
Signed-off-by: Cyrill Gorcunov <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/super.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 140174d..7c13ce7 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1295,6 +1295,8 @@ static int nfs_validate_mount_data(void *options,
args->namlen = data->namlen;
args->bsize = data->bsize;
args->auth_flavors[0] = data->pseudoflavor;
+ if (!args->nfs_server.hostname)
+ goto out_nomem;
/*
* The legacy version 6 binary mount data from userspace has a
@@ -1341,6 +1343,8 @@ static int nfs_validate_mount_data(void *options,
len = c - dev_name;
/* N.B. caller will free nfs_server.hostname in all cases */
args->nfs_server.hostname = kstrndup(dev_name, len, GFP_KERNEL);
+ if (!args->nfs_server.hostname)
+ goto out_nomem;
c++;
if (strlen(c) > NFS_MAXPATHLEN)
@@ -1384,6 +1388,10 @@ out_v3_not_compiled:
return -EPROTONOSUPPORT;
#endif /* !CONFIG_NFS_V3 */
+out_nomem:
+ dfprintk(MOUNT, "NFS: not enough memory to handle mount options\n");
+ return -ENOMEM;
+
out_no_address:
dfprintk(MOUNT, "NFS: mount program didn't pass remote address\n");
return -EINVAL;
@@ -1890,12 +1898,16 @@ static int nfs4_validate_mount_data(void *options,
return -ENAMETOOLONG;
/* N.B. caller will free nfs_server.hostname in all cases */
args->nfs_server.hostname = kstrndup(dev_name, len, GFP_KERNEL);
+ if (!args->nfs_server.hostname)
+ goto out_nomem;
c++; /* step over the ':' */
len = strlen(c);
if (len > NFS4_MAXPATHLEN)
return -ENAMETOOLONG;
args->nfs_server.export_path = kstrndup(c, len, GFP_KERNEL);
+ if (!args->nfs_server.export_path)
+ goto out_nomem;
dprintk("NFS: MNTPATH: '%s'\n", args->nfs_server.export_path);
@@ -1917,6 +1929,10 @@ out_inval_auth:
data->auth_flavourlen);
return -EINVAL;
+out_nomem:
+ dfprintk(MOUNT, "NFS4: not enough memory to handle mount options\n");
+ return -ENOMEM;
+
out_no_address:
dfprintk(MOUNT, "NFS4: mount program didn't pass remote address\n");
return -EINVAL;
On Apr 19, 2008, at 4:40 PM, Trond Myklebust wrote:
> From: Cyrill Gorcunov <[email protected]>
>
> There is possible NULL pointer dereference if kstr[n]dup failed.
The logic in super.c and client.c shouldn't assume nfs_server.hostname
is non-NULL. Can you say where the NULL dereference might happen?
> So fix them for safety.
Note that mount_server.hostname, and nfs_server.export_path also use
kstrdup without a safety net.
I see that nfs_mount and nfs4_path_walk might have a problem if a
kstrdup failed earlier.
> Signed-off-by: Cyrill Gorcunov <[email protected]>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
>
> fs/nfs/super.c | 16 ++++++++++++++++
> 1 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 140174d..7c13ce7 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1295,6 +1295,8 @@ static int nfs_validate_mount_data(void
> *options,
> args->namlen = data->namlen;
> args->bsize = data->bsize;
> args->auth_flavors[0] = data->pseudoflavor;
> + if (!args->nfs_server.hostname)
> + goto out_nomem;
>
> /*
> * The legacy version 6 binary mount data from userspace has a
> @@ -1341,6 +1343,8 @@ static int nfs_validate_mount_data(void
> *options,
> len = c - dev_name;
> /* N.B. caller will free nfs_server.hostname in all cases */
> args->nfs_server.hostname = kstrndup(dev_name, len, GFP_KERNEL);
> + if (!args->nfs_server.hostname)
> + goto out_nomem;
>
> c++;
> if (strlen(c) > NFS_MAXPATHLEN)
> @@ -1384,6 +1388,10 @@ out_v3_not_compiled:
> return -EPROTONOSUPPORT;
> #endif /* !CONFIG_NFS_V3 */
>
> +out_nomem:
> + dfprintk(MOUNT, "NFS: not enough memory to handle mount options\n");
> + return -ENOMEM;
> +
> out_no_address:
> dfprintk(MOUNT, "NFS: mount program didn't pass remote address\n");
> return -EINVAL;
> @@ -1890,12 +1898,16 @@ static int nfs4_validate_mount_data(void
> *options,
> return -ENAMETOOLONG;
> /* N.B. caller will free nfs_server.hostname in all cases */
> args->nfs_server.hostname = kstrndup(dev_name, len, GFP_KERNEL);
> + if (!args->nfs_server.hostname)
> + goto out_nomem;
>
> c++; /* step over the ':' */
> len = strlen(c);
> if (len > NFS4_MAXPATHLEN)
> return -ENAMETOOLONG;
> args->nfs_server.export_path = kstrndup(c, len, GFP_KERNEL);
> + if (!args->nfs_server.export_path)
> + goto out_nomem;
>
> dprintk("NFS: MNTPATH: '%s'\n", args->nfs_server.export_path);
>
> @@ -1917,6 +1929,10 @@ out_inval_auth:
> data->auth_flavourlen);
> return -EINVAL;
>
> +out_nomem:
> + dfprintk(MOUNT, "NFS4: not enough memory to handle mount options
> \n");
> + return -ENOMEM;
> +
> out_no_address:
> dfprintk(MOUNT, "NFS4: mount program didn't pass remote address\n");
> return -EINVAL;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Mon, 2008-04-21 at 17:13 -0400, Chuck Lever wrote:
> On Apr 19, 2008, at 4:40 PM, Trond Myklebust wrote:
> > From: Cyrill Gorcunov <[email protected]>
> >
> > There is possible NULL pointer dereference if kstr[n]dup failed.
>
> The logic in super.c and client.c shouldn't assume nfs_server.hostname
> is non-NULL. Can you say where the NULL dereference might happen?
Sure it does. See for instance all those dereferences of
nfs_client->cl_hostname. It has never been acceptable to set a null
hostname.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com