linux-kernel - Re: [PATCH 1/2] vfs: Do not allow mnt

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]

Message-ID: <alpine.LFD.2.00.1205290807350.2144@dhcp-1-248.brq.redhat.com>
Date:	Tue, 29 May 2012 12:56:44 +0200 (CEST)
From:	Lukáš Czerner <lczerner@...hat.com>
To:	Lukas Czerner <lczerner@...hat.com>
cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	mbroz@...hat.com, Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH 1/2] vfs: Do not allow mnt_longterm to go negative


Anyone can take a look at this ? It's pretty pressing I would say
since this bug will not only disallow to unattach the loop devices
but also make it unable to actually unmount the file system!

You can reproduce it like this:

mkdir -p /tmp/namespace/root/tmp
mkdir -p /tmp/namespace/private
mount --make-rshared /
mount --rbind /tmp/namespace/root/tmp /tmp/namespace/private/

losetup /dev/loop0 /aaa
mount /dev/loop0 /mnt/tst
umount /mnt/tst

I probably need to update description as I forgot about the
--make-shared thing though...sorry about that. Will resend patch
with updated description right away.

Thanks!
-Lukas

On Fri, 25 May 2012, Lukas Czerner wrote:

> Date: Fri, 25 May 2012 17:29:35 +0200
> From: Lukas Czerner <lczerner@...hat.com>
> To: linux-kernel@...r.kernel.org
> Cc: linux-fsdevel@...r.kernel.org, mbroz@...hat.com,
>     Lukas Czerner <lczerner@...hat.com>, Al Viro <viro@...iv.linux.org.uk>
> Subject: [PATCH 1/2] vfs: Do not allow mnt_longterm to go negative
> 
> Currently when someone calls __mnt_make_shortterm() and the mnt_longterm
> is already zero, it will come negative which is exactly opposite of what
> the function should to. So fix this by decrementing mnt_longterm only in
> case that it's not zero. Note that mnt_longterm should not be touched
> directly, but rather via __mnt_make_shortterm() and mnt_make_shortterm()
> functions - the former does not have this problem.
> 
> Moreover this will fix very nasty bug which might cause file systems not
> being properly cleaned up while unmounting. Specifically it might cause
> deactivate_super() not being called at all, hence the file system would
> be still up and running even after successful unmount without user even
> noticing it.
> 
> The following scenario leads to a bug. While mounting propagate_mnt()
> would walk through propagation tree cloning the mount for every every
> mount the root propagates to. Even for mounts which parents are not
> parents of the directory you're trying to mount to. So it will bump up
> the s_active counter.
> 
> However before the propagate_mnt() is left it will clean the unneeded
> cloned mounts (those which parent root is not parent of your new mount
> point), but it will _NOT_ decrement the s_active because umount_tree()
> would decrement mnt_longterm causing it to underflow, hence
> mntput_no_expire() would bail out before actually calling mntfree() and
> releasing the super block (decreasing s_active). At this point super
> block will never get released.
> 
> User can not directly observe this (aside from seeing running file system
> processes even after unmount), but it can be easily reproduced with loop
> device.
> 
> mount --bind /tmp/one /tmp/two
> losetup /dev/loop0 /mnt/test/file0
> mount /dev/loop0 /mnt/test1
> umount /mnt/test1
> losetup -d /dev/loop0
> ^^^^^^^^^^^^^^^^^^^^^
> This will fail because the device is still busy (super block is still
> active and s_active is non zero).
> 
> This commit fixes this issue.
> 
> Signed-off-by: Lukas Czerner <lczerner@...hat.com>
> Reported-by: Milan Broz <mbroz@...hat.com>
> Cc: Al Viro <viro@...iv.linux.org.uk>
> ---
>  fs/namespace.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index e608199..e72fec7 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -632,7 +632,7 @@ static inline void __mnt_make_longterm(struct mount *mnt)
>  static inline void __mnt_make_shortterm(struct mount *mnt)
>  {
>  #ifdef CONFIG_SMP
> -	atomic_dec(&mnt->mnt_longterm);
> +	atomic_add_unless(&mnt->mnt_longterm, -1, 0);
>  #endif
>  }
>  
> 
--
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/