2009-12-12 10:08:07

by Kay Sievers

[permalink] [raw]
Subject: Re: get_sb_single() - do not pass options twice

On Thu, Nov 5, 2009 at 23:28, Kay Sievers <[email protected]> wrote:
> From: Kay Sievers <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Kay Sievers <[email protected]>
> ---
>  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);


2009-12-15 09:48:43

by Miklos Szeredi

[permalink] [raw]
Subject: Re: get_sb_single() - do not pass options twice

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 <[email protected]>
> > Signed-off-by: Kay Sievers <[email protected]>
> > ---
> >  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

2009-12-15 14:39:50

by Kay Sievers

[permalink] [raw]
Subject: Re: get_sb_single() - do not pass options twice

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 <[email protected]>
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 <[email protected]>
Cc: Miklos Szeredi <[email protected]>
Signed-off-by: Kay Sievers <[email protected]>
---
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;
}