Re: get_sb_single() - do not pass options twice

Re: get_sb_single() - do not pass options twice

[Kay Sievers]

On Thu, Nov 5, 2009 at 23:28, Kay Sievers <kay.sievers@vrfy.org> wrote:
> From: Kay Sievers <kay.sievers@vrfy.org>
> Subject: vfs: get_sb_single() - do not pass options twice
>
> Filesystem code usually destroys the option buffer while
> parsing it, which leads to errors when the same buffer is
> passed twice. In case we fill a new superblock with the
> options, do not pass the options again to the remount()
> call.

Ping! Can someone please have a look and comment on that?
Something like this will now be needed for 2.6.33 to silent a warning.

Thanks,
Kay

> Cc: Greg KH <greg@kroah.com>
> Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
> ---
>  fs/super.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -900,6 +900,8 @@ int get_sb_single(struct file_system_typ
>                        deactivate_locked_super(s);
>                        return error;
>                }
> +               /* options usually get mangled and can only be parsed once */
> +               data = NULL;
>                s->s_flags |= MS_ACTIVE;
>        }
>        do_remount_sb(s, flags, data, 0);

Re: get_sb_single() - do not pass options twice

[Miklos Szeredi]

On Sat, 12 Dec 2009, Kay Sievers wrote:
> Ping! Can someone please have a look and comment on that?
> Something like this will now be needed for 2.6.33 to silent a warning.
> 
> Thanks,
> Kay
> 
> > Cc: Greg KH <greg@kroah.com>
> > Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
> > ---
> >  fs/super.c |    2 ++
> >  1 file changed, 2 insertions(+)
> >
> > --- a/fs/super.c
> > +++ b/fs/super.c
> > @@ -900,6 +900,8 @@ int get_sb_single(struct file_system_typ
> >                        deactivate_locked_super(s);
> >                        return error;
> >                }
> > +               /* options usually get mangled and can only be parsed once */
> > +               data = NULL;
> >                s->s_flags |= MS_ACTIVE;
> >        }
> >        do_remount_sb(s, flags, data, 0);

I think the do_remount_sb() is a NOP in that case.  So shouldn't it
rather be

	} else {
		do_remount_sb(s, flags, data, 0);
	}

?

Thanks,
Miklos

Re: get_sb_single() - do not pass options twice

[Kay Sievers]

On Tue, 2009-12-15 at 10:48 +0100, Miklos Szeredi wrote:
> On Sat, 12 Dec 2009, Kay Sievers wrote:
> > Ping! Can someone please have a look and comment on that?
> > Something like this will now be needed for 2.6.33 to silent a warning.

> > > --- a/fs/super.c
> > > +++ b/fs/super.c
> > > @@ -900,6 +900,8 @@ int get_sb_single(struct file_system_typ
> > >                        deactivate_locked_super(s);
> > >                        return error;
> > >                }
> > > +               /* options usually get mangled and can only be parsed once */
> > > +               data = NULL;
> > >                s->s_flags |= MS_ACTIVE;
> > >        }
> > >        do_remount_sb(s, flags, data, 0);
> 
> I think the do_remount_sb() is a NOP in that case.  So shouldn't it
> rather be
> 
> 	} else {
> 		do_remount_sb(s, flags, data, 0);
> 	}

Yeah, sounds good to me. I wasn't sure if this was done for some
non-obvious reason. In case we can do it this way, here is the patch.

Thanks,
Kay


From: Kay Sievers <kay.sievers@vrfy.org>
Subject: vfs: get_sb_single() - do not pass options twice

Filesystem code usually destroys the option buffer while
parsing it. This leads to errors when the same buffer is
passed twice. In case we fill a new superblock do not call
remount.

Cc: Greg KH <greg@kroah.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
---
 fs/super.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/fs/super.c
+++ b/fs/super.c
@@ -901,8 +901,9 @@ int get_sb_single(struct file_system_typ
 			return error;
 		}
 		s->s_flags |= MS_ACTIVE;
+	} else {
+		do_remount_sb(s, flags, data, 0);
 	}
-	do_remount_sb(s, flags, data, 0);
 	simple_set_mnt(mnt, s);
 	return 0;
 }