[PATCH] NFS: Fix handling of commmand line options for NFSROOT

[PATCH] NFS: Fix handling of commmand line options for NFSROOT

[Mark Brown]

Commmit 972903 (NFS: NFSROOT should default to "proto=udp") added a
default NFS mount option using the existing in-kernel code to parse
mount options out of the command line. This exposed a bug in the
existing code - it would not add a , separating the existing options
from the new options resulting in strings such as "udprsize=1024"
rather than "udp,rsize=1024"

Fix this by adding a , before the new options if the options are not
empty when we append.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 fs/nfs/nfsroot.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfsroot.c b/fs/nfs/nfsroot.c
index 0bc654f..309df59 100644
--- a/fs/nfs/nfsroot.c
+++ b/fs/nfs/nfsroot.c
@@ -193,10 +193,15 @@ static int __init root_nfs_parse_options(char *incoming, char *exppath,
 	 * @incoming now points to the rest of the string; if it
 	 * contains something, append it to our root options buffer
 	 */
-	if (incoming != NULL && *incoming != '\0')
+	if (incoming != NULL && *incoming != '\0') {
+		if (nfs_root_options[0] != '\0' &&
+		    root_nfs_cat(nfs_root_options, ",",
+						sizeof(nfs_root_options)))
+			return -1;
 		if (root_nfs_cat(nfs_root_options, incoming,
 						sizeof(nfs_root_options)))
 			return -1;
+	}
 
 	/*
 	 * Possibly prepare for more options to be appended
-- 
1.7.2.3

Re: [PATCH] NFS: Fix handling of commmand line options for NFSROOT

[Chuck Lever]

On Mar 10, 2011, at 10:43 AM, Mark Brown wrote:

> Commmit 972903 (NFS: NFSROOT should default to "proto=udp") added a
> default NFS mount option using the existing in-kernel code to parse
> mount options out of the command line. This exposed a bug in the
> existing code - it would not add a , separating the existing options
> from the new options resulting in strings such as "udprsize=1024"
> rather than "udp,rsize=1024"
> 
> Fix this by adding a , before the new options if the options are not
> empty when we append.

Looks like a real bug.

We might be better off teaching root_nfs_cat() to deal with the extra comma as necessary right when we concatenate the options strings.  That would simplify it's callers.

> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> fs/nfs/nfsroot.c |    7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/nfs/nfsroot.c b/fs/nfs/nfsroot.c
> index 0bc654f..309df59 100644
> --- a/fs/nfs/nfsroot.c
> +++ b/fs/nfs/nfsroot.c
> @@ -193,10 +193,15 @@ static int __init root_nfs_parse_options(char *incoming, char *exppath,
> 	 * @incoming now points to the rest of the string; if it
> 	 * contains something, append it to our root options buffer
> 	 */
> -	if (incoming != NULL && *incoming != '\0')
> +	if (incoming != NULL && *incoming != '\0') {
> +		if (nfs_root_options[0] != '\0' &&
> +		    root_nfs_cat(nfs_root_options, ",",
> +						sizeof(nfs_root_options)))
> +			return -1;
> 		if (root_nfs_cat(nfs_root_options, incoming,
> 						sizeof(nfs_root_options)))
> 			return -1;
> +	}
> 
> 	/*
> 	 * Possibly prepare for more options to be appended
> -- 
> 1.7.2.3
> 

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH] NFS: Fix handling of commmand line options for NFSROOT

[Mark Brown]

On Thu, Mar 10, 2011 at 10:59:24AM -0500, Chuck Lever wrote:
> On Mar 10, 2011, at 10:43 AM, Mark Brown wrote:

> > Commmit 972903 (NFS: NFSROOT should default to "proto=udp") added a
> > default NFS mount option using the existing in-kernel code to parse
> > mount options out of the command line. This exposed a bug in the

> Looks like a real bug.

Well, it's stopped me booting any of my systems with -next for a couple
of days :)

> We might be better off teaching root_nfs_cat() to deal with the extra
> comma as necessary right when we concatenate the options strings.
> That would simplify it's callers.

The thought did cross my mind, yes.

Re: [PATCH] NFS: Fix handling of commmand line options for NFSROOT

[Chuck Lever]

On Mar 10, 2011, at 11:07 AM, Mark Brown wrote:

> On Thu, Mar 10, 2011 at 10:59:24AM -0500, Chuck Lever wrote:
>> On Mar 10, 2011, at 10:43 AM, Mark Brown wrote:
> 
>>> Commmit 972903 (NFS: NFSROOT should default to "proto=udp") added a
>>> default NFS mount option using the existing in-kernel code to parse
>>> mount options out of the command line. This exposed a bug in the
> 
>> Looks like a real bug.
> 
> Well, it's stopped me booting any of my systems with -next for a couple
> of days :)

Thanks for your testing.

>> We might be better off teaching root_nfs_cat() to deal with the extra
>> comma as necessary right when we concatenate the options strings.
>> That would simplify it's callers.
> 
> The thought did cross my mind, yes.

That sounds like you'd like me to volunteer to do that.  :-)

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com