Re: [PATCH v3] fs: bound mount propagation fix

[Roman Borisov <ext-roman.borisov@nokia.com>]

On 04/22/2011 12:04 AM, ext Andrew Morton wrote:
> On Thu, 21 Apr 2011 16:37:28 +0400
> Roman Borisov<ext-roman.borisov@nokia.com>  wrote:
>
>> MS_SILENT flag cleaning up added to flags_to_propagation_type function.
>> It was reported that bound mount propagation doesn't work in busybox as by
>> default busybox mount applet sets the MS_SILENT flag for any mount operation.
>> Moreover recently added flags_to_propagation_type function doesn't allow to
>> do such operations as --make-[r]private --make-[r]shared etc. when MS_SILENT
>> is on.
>> The idea to clean MS_SILENT flag belongs to Denys Vlasenko<vda.linux@googlemail.com>
>>
>
> This is not an adequate changelog, IMO.  It has almost no description
> of the errant user-visible kernel behaviour and almost no description
> of what was wrong with the kernel code - ie, why the kernel was doing
> the wrong thing.
>
Hello,

I understand that description I've sent is not so clear for end-user. 
But there are two issues:
1. This is busybox related issue; only in busybox MS_SILENT is on for 
all mount operations; I don't know how to reproduce it in 
util-linux->mount without hardcoding;
2. initial scenario didn't consider "VFS: Sanity check mount flags 
passed to change_mnt_propagation()" patch (commit 
7a2e8a8faab76386d8eaae9ded739ee5615be174);

> I've pieced together a fix for the first problem but then got lazy.
Previously we used such script:

# mkdir -p mount.dir mount.shared1 mount.shared2
# touch mount.dir/a mount.dir/b
# mount -vv --bind mount.shared1 mount.shared1
# mount -vv --make-rshared mount.shared1
# mount -vv --bind mount.shared2 mount.shared2
# mount -vv --make-rshared mount.shared2
# mount -vv --bind mount.shared2 mount.shared1
# mount -vv --bind mount.dir     mount.shared2
# ls -R mount.dir mount.shared1 mount.shared2
mount.dir:
a  b
mount.shared1:
mount.shared2:
a  b
#

but expected result is:

# ls -R mount.dir mount.shared1 mount.shared2
mount.dir:
a  b
mount.shared1:
a  b
mount.shared2:
a  b
#

My "[PATCH] fs: bound mount propagation fix" fixed this issue but as I 
said I didn't consider Valerie's patch;

After I've applied the "VFS: Sanity check mount flags passed to 
change_mnt_propagation()" patch I have *another wrong result*:

# mkdir -p mount.dir mount.shared1 mount.shared2
# touch mount.dir/a mount.dir/b
# mount -vv --bind mount.shared1 mount.shared1
# mount -vv --make-rshared mount.shared1
mount: mount.shared1: Invalid argument
#

> Please can someone provide an analysis of what was causing this bug?
>
Now the debugging shows that MS_SILENT came to 
flags_to_propagation_type() and causes the error return after the 
is_power_of_2 checking; clearing the MS_SILENT flag fixes this issue.

> And has this v3 patch been tested with Denys's reproducer?
>
Yes I've applied Valerie's and my patches and tested the scenario in 
busybox environment; the verdict is passed;
I'll post [PATCH v4] with full description today.

--
Roman