Message-ID: <B2872298-BC9C-4BFD-8C88-CED88E0B7E3A@meta.com>
Date: Tue, 15 Jul 2025 22:31:39 +0000
From: Song Liu <songliubraving@...a.com>
To: Christian Brauner <brauner@...nel.org>
CC: Paul Moore <paul@...l-moore.com>, Al Viro <viro@...iv.linux.org.uk>,
        Song
 Liu <song@...nel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-security-module@...r.kernel.org"
	<linux-security-module@...r.kernel.org>,
        "apparmor@...ts.ubuntu.com"
	<apparmor@...ts.ubuntu.com>,
        "selinux@...r.kernel.org"
	<selinux@...r.kernel.org>,
        "tomoyo-users_en@...ts.sourceforge.net"
	<tomoyo-users_en@...ts.sourceforge.net>,
        "tomoyo-users_ja@...ts.sourceforge.net"
	<tomoyo-users_ja@...ts.sourceforge.net>,
        Kernel Team <kernel-team@...a.com>,
        "andrii@...nel.org" <andrii@...nel.org>,
        "eddyz87@...il.com"
	<eddyz87@...il.com>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "martin.lau@...ux.dev"
	<martin.lau@...ux.dev>,
        "jack@...e.cz" <jack@...e.cz>,
        "kpsingh@...nel.org"
	<kpsingh@...nel.org>,
        "mattbobrowski@...gle.com" <mattbobrowski@...gle.com>,
        "amir73il@...il.com" <amir73il@...il.com>,
        "repnop@...gle.com"
	<repnop@...gle.com>,
        "jlayton@...nel.org" <jlayton@...nel.org>,
        "josef@...icpanda.com" <josef@...icpanda.com>,
        "mic@...ikod.net"
	<mic@...ikod.net>,
        "gnoack@...gle.com" <gnoack@...gle.com>,
        "m@...wtm.org"
	<m@...wtm.org>,
        "john.johansen@...onical.com" <john.johansen@...onical.com>,
        "john@...armor.net" <john@...armor.net>,
        "stephen.smalley.work@...il.com"
	<stephen.smalley.work@...il.com>,
        "omosnace@...hat.com"
	<omosnace@...hat.com>,
        "takedakn@...data.co.jp" <takedakn@...data.co.jp>,
        "penguin-kernel@...ove.sakura.ne.jp" <penguin-kernel@...ove.sakura.ne.jp>,
        "enlightened@...omium.org" <enlightened@...omium.org>
Subject: Re: [RFC] vfs: security: Parse dev_name before calling
 security_sb_mount


> On Jul 15, 2025, at 3:18 AM, Christian Brauner <brauner@...nel.org> wrote:
> On Mon, Jul 14, 2025 at 03:10:57PM +0000, Song Liu wrote:


[...]

>>> If you place a new security hook into __do_loopback() the only thing
>>> that I'm not excited about is that we're holding the global namespace
>>> semaphore at that point. And I want to have as little LSM hook calls
>>> under the namespace semaphore as possible.
>> 
>> do_loopback() changed a bit since [1]. But if we put the new hook 
>> in do_loopback() before lock_mount(), we don’t have the problem with
>> the namespace semaphore, right? Also, this RFC doesn’t seem to have 
>> this issue either.
> 
> While the mount isn't locked another mount can still be mounted on top
> of it. lock_mount() will detect this and lookup the topmost mount and
> use that. IOW, the value of old_path->mnt may have changed after
> lock_mount().

I am probably confused. Do you mean path->mnt (instead of old_path->mnt) 
may have changed after lock_mount()? 

> If you have 1000 containers each calling into
>>> security_something_something_bind_mount() and then you do your "walk
>>> upwards towards the root stuff" and that root is 100000 directories away
>>> you've introduced a proper DOS or at least a severe new bottleneck into
>>> the system. And because of mount namespace propagation that needs to be
>>> serialized across all mount namespaces the namespace semaphore isn't
>>> something we can just massage away.
>> 
>> AFAICT, a poorly designed LSM can easily DoS a system. Therefore, I 
>> don’t think we need to overthink about a LSM helper causing DoS in 
>> some special scenarios. The owner of the LSM, either built-in LSM or 
>> BPF LSM, need to be aware of such risks and design the LSM rules 
>> properly to avoid DoS risks. For example, if the path tree is really 
>> deep, the LSM may decide to block the mount after walking a preset 
>> number of steps.
> 
> The scope of the lock matters _a lot_. If a poorly designed LSM happens
> to take exorbitant amount of time under the inode_lock() it's annoying:
> to anyone else wanting to grab the inode_lock() _for that single inode_.
> 
> If a poorly designed LSM does broken stuff under the namespace semaphore
> any mount event on the whole system will block, effectively deadlocking
> the system in an instant. For example, if anything even glances at
> /proc/<pid>/mountinfo it's game over. It's already iffy that we allow
> security_sb_statfs() under there but that's at least guaranteed to be
> fast.
> 
> If you can make it work so that we don't have to place security_*()
> under the namespace semaphore and you can figure out how to deal with a
> potential overmount racing you then this would be ideal for everyone.

I am trying to understand all the challenges here. 

It appears to me that do_loopback() has the tricky issue:

static int do_loopback(struct path *path, ...)
{
	...
	/* 
	 * path may still change, so not a good point to add
	 * security hook 
	 */
	mp = lock_mount(path);
	if (IS_ERR(mp)) {
		/* ... */
	}
	/* 
	 * namespace_sem is locked, so not a good point to add
	 * security hook
	 */
	...
}

Basically, without major work with locking, there is no good 
spot to insert a security hook into do_loopback(). Or, maybe 
we can add a hook somewhere in lock_mount()? 

Did I get the challenge correct?

Thanks,
Song

Powered by blists - more mailing lists