Re: NFS and kernel 2.6.x

Trond Myklebust
2004-04-19T16:41:55Z

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

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@macbeth.rhoen.de for helping debug them.

Hopefully this will be the final set of fixes.

Cheers,
Trond

[-- Attachment #2: Type: text/plain, Size: 2951 bytes --]

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;