linux-kernel - Re: [PATCH v3] fs: bound mount propagation fix

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4DB1440C.8080000@nokia.com>
Date:	Fri, 22 Apr 2011 13:02:04 +0400
From:	Roman Borisov <ext-roman.borisov@...ia.com>
To:	ext Andrew Morton <akpm@...ux-foundation.org>
CC:	viro@...iv.linux.org.uk, vda.linux@...glemail.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] fs: bound mount propagation fix

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@...ia.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@...glemail.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/