Re: [RFC] vfs: security: Parse dev_name before calling security_sb_mount

[Song Liu <songliubraving@meta.com>]

> On Jul 14, 2025, at 1:45 AM, Christian Brauner <brauner@kernel.org> wrote:
> 
> On Fri, Jul 11, 2025 at 04:22:52PM +0000, Song Liu wrote:
>> 
>> 
>>> On Jul 11, 2025, at 2:36 AM, Christian Brauner <brauner@kernel.org> wrote:
>> 
>> [...]
>> 
>>>>> 
>>>> To make sure I understand the comment. By “new mount api”, do you mean 
>>>> the code path under do_new_mount()?
>>> 
>>> fsopen()
>>> fsconfig()
>>> fsmount()
>>> open_tree()
>>> open_tree_attr()
>>> move_mount()
>>> statmount()
>>> listmount()
>>> 
>>> I think that's all.
>> 
>> Thanks for the clarification and pointer!
>> 
>>> 
>>>> 
>>>>> My recommendation is make a list of all the currently supported
>>>>> security_*() hooks in the mount code (I certainly don't have them in my
>>>>> head). Figure out what each of them allow to mediate effectively and how
>>>>> the callchains are related.
>>>>> 
>>>>> Then make a proposal how to replace them with something that a) doesn't
>>>>> cause regressions which is probably something that the LSMs care about
>>>>> and b) that covers the new mount API sufficiently to be properly
>>>>> mediated.
>>>>> 
>>>>> I'll happily review proposals. Fwiw, I'm pretty sure that this is
>>>>> something that Mickael is interested in as well.
>>>> 
>>>> So we will consider a proper redesign of LSM hooks for mount syscalls, 
>>>> but we do not want incremental improvements like this one. Do I get 
>>>> the direction right?
>>> 
>>> If incremental is workable then I think so yes. But it would be great to
>>> get a consistent picture of what people want/need.
>> 
>> In short term, we would like a way to get struct path of dev_name for  
> 
> You scared me for a second. By "dev_name" you mean the source path.

Right, we need to get struct path for the source path specified by 
string “dev_name”.

> 
>> bind mount. AFAICT, there are a few options:
>> 
>> 1. Introduce bpf_kern_path kfunc.
>> 2. Add new hook(s), such as [1].
>> 3. Something like this patch.
>> 
>> [1] https://lore.kernel.org/linux-security-module/20250110021008.2704246-1-enlightened@chromium.org/ 
>> 
>> Do you think we can ship one of them?
> 
> 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. 


> 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. 

Thanks,
Song