[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6CA51695-CB6C-42D5-80A7-8CF0E84D8F4D@meta.com>
Date: Sun, 6 Jul 2025 22:29:49 +0000
From: Song Liu <songliubraving@...a.com>
To: Mickaël Salaün <mic@...ikod.net>
CC: Song Liu <song@...nel.org>, "brauner@...nel.org" <brauner@...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>,
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>,
"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
"jack@...e.cz" <jack@...e.cz>,
"kpsingh@...nel.org" <kpsingh@...nel.org>,
"mattbobrowski@...gle.com" <mattbobrowski@...gle.com>,
"m@...wtm.org"
<m@...wtm.org>, "neil@...wn.name" <neil@...wn.name>,
Günther Noack <gnoack@...gle.com>,
Jann Horn
<jannh@...gle.com>
Subject: Re: [PATCH v5 bpf-next 2/5] landlock: Use path_walk_parent()
> On Jul 4, 2025, at 2:00 AM, Mickaël Salaün <mic@...ikod.net> wrote:
>
> On Thu, Jul 03, 2025 at 10:27:02PM +0000, Song Liu wrote:
>> Hi Mickaël,
>>
>>> On Jul 3, 2025, at 11:29 AM, Mickaël Salaün <mic@...ikod.net> wrote:
>>>
>>> On Mon, Jun 16, 2025 at 11:11:13PM -0700, Song Liu wrote:
>>>> Use path_walk_parent() to walk a path up to its parent.
>>>>
>>>> No functional changes intended.
>>>
>>> Using this helper actualy fixes the issue highlighted by Al. Even if it
>>> was reported after the first version of this patch series, the issue
>>> should be explained in the commit message and these tags should be
>>> added:
>>>
>>> Reported-by: Al Viro <viro@...iv.linux.org.uk>
>>> Closes: https://lore.kernel.org/r/20250529231018.GP2023217@ZenIV
>>> Fixes: cb2c7d1a1776 ("landlock: Support filesystem access-control")
>>>
>>> I like this new helper but we should have a clear plan to be able to
>>> call such helper in a RCU read-side critical section before we merge
>>> this series. We're still waiting for Christian.
>>>
>>> I sent a patch to fix the handling of disconnected directories for
>>> Landlock, and it will need to be backported:
>>> https://lore.kernel.org/all/20250701183812.3201231-1-mic@digikod.net/
>>> Unfortunately a rebase would be needed for the path_walk_parent patch,
>>> but I can take it in my tree if everyone is OK.
>>
>> The fix above also touches VFS code (makes path_connected available
>> out of namei.c. It probably should also go through VFS tree?
>>
>> Maybe you can send 1/5 and 2/5 of this set (with necessary changes)
>> and your fix together to VFS tree. Then, I will see how to route the
>> BPF side patches.
>
> That could work, but because it would be much more Landlock-specific
> code than VFS-specific code, and there will probably be a few versions
> of my fixes, I'd prefer to keep this into my tree if VFS folks are OK.
> BTW, my fixes already touch the VFS subsystem a bit.
>
> However, as pointed out in my previous email, the disconnected directory
> case should be carefully considered for the path_walk_parent() users to
> avoid BPF LSM programs having the same issue I'm fixing for Landlock.
> The safe approaches I can think of to avoid this issue for BPF programs
> while making the interface efficient (by not calling path_connected()
> after each path_walk_parent() call) is to either have some kind of
> iterator as Tingmao proposed, or a callback function as Neil proposed.
> The callback approach looks simpler and more future-proof, but I guess
> you'll have to make it compatible with the eBPF runtime. I think the
> best approach would be to have a VFS API with a callback, and a BPF
> helper (leveraging this VFS API) with an iterator state.
Since we are proposing an open-coded BPF iterator. Having a real
callback, which is no longer an open coded iterator, requires more
work. At the moment, it is easier to just add a path_connected call
in bpf_iter_path_next().
Thanks,
Song
Powered by blists - more mailing lists