Re: 2.6.35-rc4: mount results with and without MS_SILENT differ

[Denys Vlasenko <vda.linux@googlemail.com>]

On Friday 25 March 2011 04:47, Chuck Ebbert wrote:
> On Mon, 14 Mar 2011 05:20:08 +0100
> Denys Vlasenko <vda.linux@googlemail.com> wrote:
> 
> It looks like this bug has been there forever.
> 
> This fails:
> > mount -vv --make-rshared mount.shared1               2>&1
> 
> This succeeds:
> > mount -vv --loud --make-rshared mount.shared2               2>&1  # <-HERE
> 
> Failure case uses MS_REC | MS_SHARED | MS_SILENT
> > mount: mount('','mount.shared1','',0x0010c000,''):0
> 
> It succeeds with MS_REC | MS_SHARED
> > mount: mount('','mount.shared2','',0x00104000,''):0
> 
> Looking at do_mount() we see some flags get filtered out before checking
> namespace flags:
> 
>         flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | MS_BORN |
>                    MS_NOATIME | MS_NODIRATIME | MS_RELATIME| MS_KERNMOUNT |
>                    MS_STRICTATIME);
> 
> Then:
> 
>         else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
>                 retval = do_change_type(&path, flags);
> 
> In do_change_type()
> 
>         type = flags_to_propagation_type(flag);
>         if (!type)
>                 return -EINVAL;
> 
> And flags_to_propagation_type() filters out MS_REC before making sure one
> and only one of the propagation type flags is set:
> 
>         int type = flags & ~MS_REC;
> 
>         /* Fail if any non-propagation flags are set */
>         if (type & ~(MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
>                 return 0;
>         /* Only one propagation flag should be set */
>         if (!is_power_of_2(type))
>                 return 0;
> 
> Looks like the is_power_of_2() test is failing because MS_SILENT is set.
> I'm not sure whether to filter MS_SILENT in the upper or lower function
> though.

We cannot slear out MS_SILENT here:

        flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | MS_BORN |
                   MS_NOATIME | MS_NODIRATIME | MS_RELATIME| MS_KERNMOUNT |
                   MS_STRICTATIME);

Because...

        if (flags & MS_REMOUNT)
                retval = do_remount(&path, flags & ~MS_REMOUNT, mnt_flags,
                                    data_page);
        else if (flags & MS_BIND)
                retval = do_loopback(&path, dev_name, flags & MS_REC);
        else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
                retval = do_change_type(&path, flags);
        else if (flags & MS_MOVE)
                retval = do_move_mount(&path, dev_name);
        else
...it needs to propagate into:
                retval = do_new_mount(&path, type_page, flags, mnt_flags,
                                      dev_name, data_page);


I think MS_SILENT can be cleared along with MS_REC in do_change_type() ->
flags_to_propagation_type(), on this line:

-       int type = flags & ~MS_REC;
+       int type = flags & ~(MS_REC | MS_SILENT);


-- 
vda