Re: NFS and kernel 2.6.x

[Trond Myklebust]

On Sat, 2004-04-17 at 22:51, Trond Myklebust wrote:
> On Sat, 2004-04-17 at 12:19, Russell King wrote:
> 
> > Firstly, as far as I can see, args[] is uninitialised.  If match_token
> > doesn't touch args[] then we pass match_int some uninitialised kernel
> > memory.
> >
> > Secondly, we seem to exit if match_int doesn't parse a number.  Not
> > all options in "tokens" have a number associated with them, including
> > ones like "tcp".
> 
> Agreed. The correct fix should be something like the appended patch. It
> depends on all tokens that do take an integer argument being listed
> first in the enum.
> 
> Comments?

It turned out there were a few extra issues that weren't fixed by the
previous patch. Thanks to boris@xxxxxxxxxxxxxxxx for helping debug them.

Hopefully this will be the final set of fixes.

Cheers,
  Trond


 nfsroot.c |   33 +++++++++++++++++++++------------
 1 files changed, 21 insertions(+), 12 deletions(-)

diff -u --recursive --new-file --show-c-function linux-2.6.6-01-soft/fs/nfs/nfsroot.c linux-2.6.6-02-fix_nfsroot/fs/nfs/nfsroot.c
--- linux-2.6.6-01-soft/fs/nfs/nfsroot.c	2004-04-17 23:01:09.000000000 -0400
+++ linux-2.6.6-02-fix_nfsroot/fs/nfs/nfsroot.c	2004-04-19 12:08:31.000000000 -0400
@@ -117,11 +117,16 @@ static int mount_port __initdata = 0;		/
  ***************************************************************************/
 
 enum {
+	/* Options that take integer arguments */
 	Opt_port, Opt_rsize, Opt_wsize, Opt_timeo, Opt_retrans, Opt_acregmin,
-	Opt_acregmax, Opt_acdirmin, Opt_acdirmax, Opt_soft, Opt_hard, Opt_intr,
+	Opt_acregmax, Opt_acdirmin, Opt_acdirmax,
+	/* Options that take no arguments */
+	Opt_soft, Opt_hard, Opt_intr,
 	Opt_nointr, Opt_posix, Opt_noposix, Opt_cto, Opt_nocto, Opt_ac, 
 	Opt_noac, Opt_lock, Opt_nolock, Opt_v2, Opt_v3, Opt_udp, Opt_tcp,
-	Opt_broken_suid, Opt_err,
+	Opt_broken_suid,
+	/* Error token */
+	Opt_err
 };
 
 static match_table_t tokens = {
@@ -146,9 +151,13 @@ static match_table_t tokens = {
 	{Opt_noac, "noac"},
 	{Opt_lock, "lock"},
 	{Opt_nolock, "nolock"},
+	{Opt_v2, "nfsvers=2"},
 	{Opt_v2, "v2"},
+	{Opt_v3, "nfsvers=3"},
 	{Opt_v3, "v3"},
+	{Opt_udp, "proto=udp"},
 	{Opt_udp, "udp"},
+	{Opt_tcp, "proto=tcp"},
 	{Opt_tcp, "tcp"},
 	{Opt_broken_suid, "broken_suid"},
 	{Opt_err, NULL}
@@ -162,25 +171,21 @@ static match_table_t tokens = {
 static int __init root_nfs_parse(char *name, char *buf)
 {
 
-	char *p;
+	char *p, *path = name;
 	substring_t args[MAX_OPT_ARGS];
 	int option;
 
 	if (!name)
 		return 1;
 
-	if (name[0] && strcmp(name, "default")){
-		strlcpy(buf, name, NFS_MAXPATHLEN);
-		return 1;
-	}
 	while ((p = strsep (&name, ",")) != NULL) {
 		int token; 
 		if (!*p)
 			continue;
 		token = match_token(p, tokens, args);
 
-		/* %u tokens only */
-		if (match_int(&args[0], &option))
+		/* %u tokens only. Beware if you add new tokens! */
+		if (token < Opt_soft && match_int(&args[0], &option))
 			return 0;
 		switch (token) {
 			case Opt_port:
@@ -265,6 +270,13 @@ static int __init root_nfs_parse(char *n
 				return 0;
 		}
 	}
+
+	/*
+	 * Copy the NFS remote path to the output buffer.
+	 * Relies on strsep() having converted the delimiting ',' to '\0'.
+	 */
+	if (path[0] != '\0' && strcmp(path, "default") != 0)
+		strlcpy(buf, path, NFS_MAXPATHLEN);
 	return 1;
 }
 
@@ -283,9 +295,6 @@ static int __init root_nfs_name(char *na
 	nfs_data.flags    = NFS_MOUNT_NONLM;	/* No lockd in nfs root yet */
 	nfs_data.rsize    = NFS_DEF_FILE_IO_BUFFER_SIZE;
 	nfs_data.wsize    = NFS_DEF_FILE_IO_BUFFER_SIZE;
-	nfs_data.bsize	  = 0;
-	nfs_data.timeo    = 7;
-	nfs_data.retrans  = 3;
 	nfs_data.acregmin = 3;
 	nfs_data.acregmax = 60;
 	nfs_data.acdirmin = 30;