--- fs/nfs/inode.c.orig 2005-03-15 16:12:07.000000000 +0000
+++ fs/nfs/inode.c 2005-03-16 13:42:13.000000000 +0000
@@ -1385,9 +1385,19 @@ static struct super_block *nfs_get_sb(st
/* Zero out the NFS state stuff */
init_nfsv4_state(server);
+ /* NFS v2 and v3 are interoperable and will continue to be so..
+ * This should be reviewed when NFS_MOUNT_VERSION increments */
+ if ((data->version <2 ||
+ data->version >6 ||
+ NFS_MOUNT_VERSION < 2 ||
+ NFS_MOUNT_VERSION > 6)
+ && data->version != NFS_MOUNT_VERSION) {
+ printk("nfs warning: possibly dangerous mount version incompatibility: mount(v%d) %s than kernel(v%d)\n",
+ data->version, data->version < NFS_MOUNT_VERSION ? "older" : "newer", NFS_MOUNT_VERSION);
+ }
+
+ /* All <> values of NFS_MOUNT_VERSION 2..6 are handled */
if (data->version != NFS_MOUNT_VERSION) {
- printk("nfs warning: mount version %s than kernel\n",
- data->version < NFS_MOUNT_VERSION ? "older" : "newer");
if (data->version < 2)
data->namlen = 0;
if (data->version < 3)
On Thu, 17 Mar 2005, Trond Myklebust wrote:
> I started rethinking this subject last night in between cleanups of the
> ACL code. Attached is the resulting patch proposal. It goes a lot
> further that what you do, but should hopefully provide better error
> reporting in "mount" and clean up some of that code.
>
> In particular, note that it attempts to add an error code that tells you
> whether or not NFSv3 is actually supported by the kernel: I used
> EPROTONOSUPPORT, since that gives the correct error message with
> perror() (although strictly, EPROTONOSUPPORT appears to be a socket
> error). If anyone has a better suggestion for an error that makes more
> sense, then I'm all ears...
ENOSYS might be an alternative, though personally I have no problem with
EPROTONOSUPPORT. ENOSYS is worse because it's then hard to differentiate
between no NFS support at all and no support for that particular version.
This is actually very good news for amd, because we've been struggling for
years to detect (and work around) the case when the kernel version (and
headers) indicate that v3 support would be available on the client side,
but the user doesn't have it compiled in.
I think the patch looks good. Just a small problem with one error path:
+ s = ERR_PTR(-ENOMEM);
server = kmalloc(sizeof(struct nfs_server), GFP_KERNEL);
if (!server)
- return ERR_PTR(-ENOMEM);
+ goto out_err;
[...]
+out_err:
+ kfree(server);
+ return s;
This will call kfree(NULL) if the kmalloc fails. Which may well be fine--I
haven't looked at kfree to see if handles that case.
Thanks,
Ion
--
It is better to keep your mouth shut and be thought a fool,
than to open it and remove all doubt.
-------------------------------------------------------
This SF.net email is sponsored by Microsoft Mobile & Embedded DevCon 2005
Attend MEDC 2005 May 9-12 in Vegas. Learn more about the latest Windows
Embedded(r) & Windows Mobile(tm) platforms, applications & content. Register
by 3/29 & save $300 http://ads.osdn.com/?ad_id=6883&alloc_id=15149&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
NFS: Kill annoying mount version mismatch printks
Ensure that we fix up the missing fields in the nfs_mount_data with
sane defaults for older versions of mount, and return errors in the
cases where we cannot.
Convert a bunch of annoying warnings into dprintks()
Return -EPROTONOSUPPORT rather than EIO if mount() tries to set NFSv3
without it actually being compiled in.
Signed-off-by: Trond Myklebust <[email protected]>
---
inode.c | 179 +++++++++++++++++++++++++++++++++++++---------------------------
1 files changed, 105 insertions(+), 74 deletions(-)
Index: linux-2.6.11/fs/nfs/inode.c
===================================================================
--- linux-2.6.11.orig/fs/nfs/inode.c
+++ linux-2.6.11/fs/nfs/inode.c
@@ -366,13 +366,15 @@ nfs_create_client(struct nfs_server *ser
xprt = xprt_create_proto(tcp ? IPPROTO_TCP : IPPROTO_UDP,
&server->addr, &timeparms);
if (IS_ERR(xprt)) {
- printk(KERN_WARNING "NFS: cannot create RPC transport.\n");
+ dprintk("%s: cannot create RPC transport. Error = %ld\n",
+ __FUNCTION__, PTR_ERR(xprt));
return (struct rpc_clnt *)xprt;
}
clnt = rpc_create_client(xprt, server->hostname, &nfs_program,
server->rpc_ops->version, data->pseudoflavor);
if (IS_ERR(clnt)) {
- printk(KERN_WARNING "NFS: cannot create RPC client.\n");
+ dprintk("%s: cannot create RPC client. Error = %ld\n",
+ __FUNCTION__, PTR_ERR(xprt));
goto out_fail;
}
@@ -427,21 +429,16 @@ nfs_fill_super(struct super_block *sb, s
/* Check NFS protocol revision and initialize RPC op vector
* and file handle pool. */
- if (server->flags & NFS_MOUNT_VER3) {
#ifdef CONFIG_NFS_V3
+ if (server->flags & NFS_MOUNT_VER3) {
server->rpc_ops = &nfs_v3_clientops;
server->caps |= NFS_CAP_READDIRPLUS;
- if (data->version < 4) {
- printk(KERN_NOTICE "NFS: NFSv3 not supported by mount program.\n");
- return -EIO;
- }
-#else
- printk(KERN_NOTICE "NFS: NFSv3 not supported.\n");
- return -EIO;
-#endif
} else {
server->rpc_ops = &nfs_v2_clientops;
}
+#else
+ server->rpc_ops = &nfs_v2_clientops;
+#endif
/* Fill in pseudoflavor for mount version < 5 */
if (!(data->flags & NFS_MOUNT_SECFLAVOUR))
@@ -1385,74 +1382,94 @@ static struct super_block *nfs_get_sb(st
int flags, const char *dev_name, void *raw_data)
{
int error;
- struct nfs_server *server;
+ struct nfs_server *server = NULL;
struct super_block *s;
struct nfs_fh *root;
struct nfs_mount_data *data = raw_data;
- if (!data) {
- printk("nfs_read_super: missing data argument\n");
- return ERR_PTR(-EINVAL);
+ s = ERR_PTR(-EINVAL);
+ if (data == NULL) {
+ dprintk("%s: missing data argument\n", __FUNCTION__);
+ goto out_err;
+ }
+ if (data->version <= 0 || data->version > NFS_MOUNT_VERSION) {
+ dprintk("%s: bad mount version\n", __FUNCTION__);
+ goto out_err;
}
+ switch (data->version) {
+ case 1:
+ data->namlen = 0;
+ case 2:
+ data->bsize = 0;
+ case 3:
+ if (data->flags & NFS_MOUNT_VER3) {
+ dprintk("%s: mount structure version %d does not support NFSv3\n",
+ __FUNCTION__,
+ data->version);
+ goto out_err;
+ }
+ data->root.size = NFS2_FHSIZE;
+ memcpy(data->root.data, data->old_root.data, NFS2_FHSIZE);
+ case 4:
+ if (data->flags & NFS_MOUNT_SECFLAVOUR) {
+ dprintk("%s: mount structure version %d does not support strong security\n",
+ __FUNCTION__,
+ data->version);
+ goto out_err;
+ }
+ case 5:
+ memset(data->context, 0, sizeof(data->context));
+ }
+#ifndef CONFIG_NFS_V3
+ /* If NFSv3 is not compiled in, return -EPROTONOSUPPORT */
+ s = ERR_PTR(-EPROTONOSUPPORT);
+ if (data->flags & NFS_MOUNT_VER3) {
+ dprintk("%s: NFSv3 not compiled into kernel\n", __FUNCTION__);
+ goto out_err;
+ }
+#endif /* CONFIG_NFS_V3 */
+ s = ERR_PTR(-ENOMEM);
server = kmalloc(sizeof(struct nfs_server), GFP_KERNEL);
if (!server)
- return ERR_PTR(-ENOMEM);
+ goto out_err;
memset(server, 0, sizeof(struct nfs_server));
/* Zero out the NFS state stuff */
init_nfsv4_state(server);
- if (data->version != NFS_MOUNT_VERSION) {
- printk("nfs warning: mount version %s than kernel\n",
- data->version < NFS_MOUNT_VERSION ? "older" : "newer");
- if (data->version < 2)
- data->namlen = 0;
- if (data->version < 3)
- data->bsize = 0;
- if (data->version < 4) {
- data->flags &= ~NFS_MOUNT_VER3;
- data->root.size = NFS2_FHSIZE;
- memcpy(data->root.data, data->old_root.data, NFS2_FHSIZE);
- }
- if (data->version < 5)
- data->flags &= ~NFS_MOUNT_SECFLAVOUR;
- }
-
root = &server->fh;
if (data->flags & NFS_MOUNT_VER3)
root->size = data->root.size;
else
root->size = NFS2_FHSIZE;
+ s = ERR_PTR(-EINVAL);
if (root->size > sizeof(root->data)) {
- printk("nfs_get_sb: invalid root filehandle\n");
- kfree(server);
- return ERR_PTR(-EINVAL);
+ dprintk("%s: invalid root filehandle\n", __FUNCTION__);
+ goto out_err;
}
memcpy(root->data, data->root.data, root->size);
/* We now require that the mount process passes the remote address */
memcpy(&server->addr, &data->addr, sizeof(server->addr));
if (server->addr.sin_addr.s_addr == INADDR_ANY) {
- printk("NFS: mount program didn't pass remote address!\n");
- kfree(server);
- return ERR_PTR(-EINVAL);
+ dprintk("%s: mount program didn't pass remote address!\n",
+ __FUNCTION__);
+ goto out_err;
}
- s = sget(fs_type, nfs_compare_super, nfs_set_super, server);
-
- if (IS_ERR(s) || s->s_root) {
- kfree(server);
- return s;
+ /* Fire up rpciod if not yet running */
+ s = ERR_PTR(rpciod_up());
+ if (IS_ERR(s)) {
+ dprintk("%s: couldn't start rpciod! Error = %ld\n",
+ __FUNCTION__, PTR_ERR(s));
+ goto out_err;
}
- s->s_flags = flags;
+ s = sget(fs_type, nfs_compare_super, nfs_set_super, server);
+ if (IS_ERR(s) || s->s_root)
+ goto out_rpciod_down;
- /* Fire up rpciod if not yet running */
- if (rpciod_up() != 0) {
- printk(KERN_WARNING "NFS: couldn't start rpciod!\n");
- kfree(server);
- return ERR_PTR(-EIO);
- }
+ s->s_flags = flags;
error = nfs_fill_super(s, data, flags & MS_VERBOSE ? 1 : 0);
if (error) {
@@ -1462,6 +1479,11 @@ static struct super_block *nfs_get_sb(st
}
s->s_flags |= MS_ACTIVE;
return s;
+out_rpciod_down:
+ rpciod_down();
+out_err:
+ kfree(server);
+ return s;
}
static void nfs_kill_super(struct super_block *s)
@@ -1594,15 +1616,19 @@ static int nfs4_fill_super(struct super_
clp = nfs4_get_client(&server->addr.sin_addr);
if (!clp) {
- printk(KERN_WARNING "NFS: failed to create NFS4 client.\n");
+ dprintk("%s: failed to create NFS4 client.\n", __FUNCTION__);
return -EIO;
}
/* Now create transport and client */
authflavour = RPC_AUTH_UNIX;
if (data->auth_flavourlen != 0) {
- if (data->auth_flavourlen > 1)
- printk(KERN_INFO "NFS: cannot yet deal with multiple auth flavours.\n");
+ if (data->auth_flavourlen != 1) {
+ dprintk("%s: Invalid number of RPC auth flavours %d.\n",
+ __FUNCTION__, data->auth_flavourlen);
+ err = -EINVAL;
+ goto out_fail;
+ }
if (copy_from_user(&authflavour, data->auth_flavours, sizeof(authflavour))) {
err = -EFAULT;
goto out_fail;
@@ -1614,17 +1640,19 @@ static int nfs4_fill_super(struct super_
xprt = xprt_create_proto(proto, &server->addr, &timeparms);
if (IS_ERR(xprt)) {
up_write(&clp->cl_sem);
- printk(KERN_WARNING "NFS: cannot create RPC transport.\n");
err = PTR_ERR(xprt);
+ dprintk("%s: cannot create RPC transport. Error = %d\n",
+ __FUNCTION__, err);
goto out_fail;
}
clnt = rpc_create_client(xprt, server->hostname, &nfs_program,
server->rpc_ops->version, authflavour);
if (IS_ERR(clnt)) {
up_write(&clp->cl_sem);
- printk(KERN_WARNING "NFS: cannot create RPC client.\n");
xprt_destroy(xprt);
err = PTR_ERR(clnt);
+ dprintk("%s: cannot create RPC client. Error = %d\n",
+ __FUNCTION__, err);
goto out_fail;
}
clnt->cl_intr = 1;
@@ -1656,20 +1684,22 @@ static int nfs4_fill_super(struct super_
clp = NULL;
if (IS_ERR(clnt)) {
- printk(KERN_WARNING "NFS: cannot create RPC client.\n");
- return PTR_ERR(clnt);
+ err = PTR_ERR(clnt);
+ dprintk("%s: cannot create RPC client. Error = %d\n",
+ __FUNCTION__, err);
+ return err;
}
server->client = clnt;
if (server->nfs4_state->cl_idmap == NULL) {
- printk(KERN_WARNING "NFS: failed to create idmapper.\n");
+ dprintk("%s: failed to create idmapper.\n", __FUNCTION__);
return -ENOMEM;
}
if (clnt->cl_auth->au_flavor != authflavour) {
if (rpcauth_create(authflavour, clnt) == NULL) {
- printk(KERN_WARNING "NFS: couldn't create credcache!\n");
+ dprintk("%s: couldn't create credcache!\n", __FUNCTION__);
return -ENOMEM;
}
}
@@ -1730,8 +1760,12 @@ static struct super_block *nfs4_get_sb(s
struct nfs4_mount_data *data = raw_data;
void *p;
- if (!data) {
- printk("nfs_read_super: missing data argument\n");
+ if (data == NULL) {
+ dprintk("%s: missing data argument\n", __FUNCTION__);
+ return ERR_PTR(-EINVAL);
+ }
+ if (data->version <= 0 || data->version > NFS4_MOUNT_VERSION) {
+ dprintk("%s: bad mount version\n", __FUNCTION__);
return ERR_PTR(-EINVAL);
}
@@ -1742,11 +1776,6 @@ static struct super_block *nfs4_get_sb(s
/* Zero out the NFS state stuff */
init_nfsv4_state(server);
- if (data->version != NFS4_MOUNT_VERSION) {
- printk("nfs warning: mount version %s than kernel\n",
- data->version < NFS4_MOUNT_VERSION ? "older" : "newer");
- }
-
p = nfs_copy_user_string(NULL, &data->hostname, 256);
if (IS_ERR(p))
goto out_err;
@@ -1773,11 +1802,20 @@ static struct super_block *nfs4_get_sb(s
}
if (server->addr.sin_family != AF_INET ||
server->addr.sin_addr.s_addr == INADDR_ANY) {
- printk("NFS: mount program didn't pass remote IP address!\n");
+ dprintk("%s: mount program didn't pass remote IP address!\n",
+ __FUNCTION__);
s = ERR_PTR(-EINVAL);
goto out_free;
}
+ /* Fire up rpciod if not yet running */
+ s = ERR_PTR(rpciod_up());
+ if (IS_ERR(s)) {
+ dprintk("%s: couldn't start rpciod! Error = %ld\n",
+ __FUNCTION__, PTR_ERR(s));
+ goto out_free;
+ }
+
s = sget(fs_type, nfs4_compare_super, nfs_set_super, server);
if (IS_ERR(s) || s->s_root)
@@ -1785,13 +1823,6 @@ static struct super_block *nfs4_get_sb(s
s->s_flags = flags;
- /* Fire up rpciod if not yet running */
- if (rpciod_up() != 0) {
- printk(KERN_WARNING "NFS: couldn't start rpciod!\n");
- s = ERR_PTR(-EIO);
- goto out_free;
- }
-
error = nfs4_fill_super(s, data, flags & MS_VERBOSE ? 1 : 0);
if (error) {
up_write(&s->s_umount);
m=E5 den 21.03.2005 Klokka 11:18 (-0500) skreiv Ion Badulescu:
> On Thu, 17 Mar 2005, Trond Myklebust wrote:
>=20
> > I started rethinking this subject last night in between cleanups of the
> > ACL code. Attached is the resulting patch proposal. It goes a lot
> > further that what you do, but should hopefully provide better error
> > reporting in "mount" and clean up some of that code.
> >
> > In particular, note that it attempts to add an error code that tells yo=
u
> > whether or not NFSv3 is actually supported by the kernel: I used
> > EPROTONOSUPPORT, since that gives the correct error message with
> > perror() (although strictly, EPROTONOSUPPORT appears to be a socket
> > error). If anyone has a better suggestion for an error that makes more
> > sense, then I'm all ears...
>=20
> ENOSYS might be an alternative, though personally I have no problem with=20
> EPROTONOSUPPORT. ENOSYS is worse because it's then hard to differentiate=20
> between no NFS support at all and no support for that particular version.
Right. The problem is that ENOSYS is supposed to mean "function not
implemented", but is usually interpreted in userland to mean "syscall
not implemented".
The thing that scared me away from ENOSYS was that "umount" is actually
using it in order to figure out whether or not it can use sys_umount2().
> This is actually very good news for amd, because we've been struggling fo=
r=20
> years to detect (and work around) the case when the kernel version (and=20
> headers) indicate that v3 support would be available on the client side,=20
> but the user doesn't have it compiled in.
>=20
> I think the patch looks good. Just a small problem with one error path:
>=20
> + s =3D ERR_PTR(-ENOMEM);
> server =3D kmalloc(sizeof(struct nfs_server), GFP_KERNEL);
> if (!server)
> - return ERR_PTR(-ENOMEM);
> + goto out_err;
> [...]
> +out_err:
> + kfree(server);
> + return s;
>=20
> This will call kfree(NULL) if the kmalloc fails. Which may well be fine--=
I=20
> haven't looked at kfree to see if handles that case.
It is safe. kfree() automatically detects NULL pointers.
Cheers,
Trond
--=20
Trond Myklebust <[email protected]>
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs