linux-kernel - Re: [PATCH] fs/pnode.c: treat zero mnt_group

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 16 Feb 2016 23:07:32 -0800
From:	Maxim Patlasov <mpatlasov@...tuozzo.com>
To:	Al Viro <viro@...IV.linux.org.uk>
CC:	<linux-fsdevel@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<devel@...nvz.org>
Subject: Re: [PATCH] fs/pnode.c: treat zero mnt_group_id-s as unequal

On 02/16/2016 11:54 AM, Al Viro wrote:
> On Tue, Feb 16, 2016 at 11:45:33AM -0800, Maxim Patlasov wrote:
>> propagate_one(m) calculates "type" argument for copy_tree() like this:
>>
>>>     if (m->mnt_group_id == last_dest->mnt_group_id) {
>>>         type = CL_MAKE_SHARED;
>>>     } else {
>>>         type = CL_SLAVE;
>>>         if (IS_MNT_SHARED(m))
>>>            type |= CL_MAKE_SHARED;
>>>    }
>> The "type" argument then governs clone_mnt() behavior with respect to flags
>> and mnt_master of new mount. When we iterate through a slave group, it is
>> possible that both current "m" and "last_dest" are not shared (although,
>> both are slaves, i.e. have non-NULL mnt_master-s). Then the comparison
>> above erroneously makes new mount shared and sets its mnt_master to
>> last_source->mnt_master. The patch fixes the problem by handling zero
>> mnt_group_id-s as though they are unequal.
>>
>> The similar problem exists in the implementation of "else" clause above
>> when we have to ascend upward in the master/slave tree by calling:
>>
>>>     last_source = last_source->mnt_master;
>>>     last_dest = last_source->mnt_parent;
>> proper number of times. The last step is governed by
>> "n->mnt_group_id != last_dest->mnt_group_id" condition that may lie if
>> both are zero. The patch fixes this case in the same way as the former one.
> Mind putting together a reproducer?

There are two files attached: reproducer1.c and reproducer2.c. The 
former demonstrates the problem before applying the patch. The latter 
demonstrates why the first hunk of the patch is not enough.

[root@...ml ~]# reproducer1
main pid = 1496
monitor pid = 1497
child pid = 1498
grand-child pid = 1499

[root@...ml ~]# grep "child" /proc/1496/mountinfo
243 144 0:37 /child /tmp/child rw shared:93 - tmpfs tmpfs rw,seclabel
[root@...ml ~]# grep "child" /proc/1498/mountinfo
244 208 0:37 /child /tmp/child rw shared:127 master:93 - tmpfs tmpfs 
rw,seclabel
[root@...ml ~]# grep "child" /proc/1499/mountinfo
245 240 0:37 /child /tmp/child rw master:127 - tmpfs tmpfs rw,seclabel
[root@...ml ~]# grep "child" /proc/1497/mountinfo
246 176 0:37 /child /tmp/child rw shared:128 master:127 - tmpfs tmpfs 
rw,seclabel

while expected info for 1497 would be:
246 176 0:37 /child /tmp/child rw master:93 - tmpfs tmpfs rw,seclabel

Now, assuming that only the first hunk of the patch is applied:

 > -    if (m->mnt_group_id == last_dest->mnt_group_id) {
 > +    if (m->mnt_group_id && m->mnt_group_id == last_dest->mnt_group_id) {

[root@...ml ~]# reproducer2
main pid = 1506
monitor pid = 1507
child pid = 1508
grand-child pid = 1509

[root@...ml ~]# grep "child" /proc/1506/mountinfo
243 144 0:37 /child /tmp/child rw shared:93 - tmpfs tmpfs rw,seclabel
[root@...ml ~]# grep "child" /proc/1508/mountinfo
244 208 0:37 /child /tmp/child rw shared:93 - tmpfs tmpfs rw,seclabel
[root@...ml ~]# grep "child" /proc/1509/mountinfo
245 240 0:37 /child /tmp/child rw master:93 - tmpfs tmpfs rw,seclabel
[root@...ml ~]# grep "child" /proc/1507/mountinfo
246 176 0:37 /child /tmp/child rw master:0 - tmpfs tmpfs rw,seclabel

while expected info for 1507 would be:
246 176 0:37 /child /tmp/child rw master:93 - tmpfs tmpfs rw,seclabel

Thanks,
Maxim

View attachment "reproducer1.c" of type "text/x-csrc" (2440 bytes)

View attachment "reproducer2.c" of type "text/x-csrc" (2210 bytes)