| 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
| ||
|
Message-ID: <58FB95C4-1499-4865-8FA7-3E1F64EB5EDE@meta.com>
Date: Sun, 6 Jul 2025 23:54:58 +0000
From: Song Liu <songliubraving@...a.com>
To: Yonghong Song <yonghong.song@...ux.dev>
CC: 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>,
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>,
"brauner@...nel.org" <brauner@...nel.org>,
"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>
Subject: Re: [PATCH v5 bpf-next 1/5] namei: Introduce new helper function
path_walk_parent()
> On Jul 4, 2025, at 10:40 AM, Yonghong Song <yonghong.song@...ux.dev> wrote:
[...]
>> +static struct dentry *__path_walk_parent(struct path *path, const struct path *root, int flags)
>> {
>> - struct dentry *parent;
>> -
>> - if (path_equal(&nd->path, &nd->root))
>> + if (path_equal(path, root))
>> goto in_root;
>> - if (unlikely(nd->path.dentry == nd->path.mnt->mnt_root)) {
>> - struct path path;
>> + if (unlikely(path->dentry == path->mnt->mnt_root)) {
>> + struct path new_path;
>> - if (!choose_mountpoint(real_mount(nd->path.mnt),
>> - &nd->root, &path))
>> + if (!choose_mountpoint(real_mount(path->mnt),
>> + root, &new_path))
>> goto in_root;
>> - path_put(&nd->path);
>> - nd->path = path;
>> - nd->inode = path.dentry->d_inode;
>> - if (unlikely(nd->flags & LOOKUP_NO_XDEV))
>> + path_put(path);
>> + *path = new_path;
>> + if (unlikely(flags & LOOKUP_NO_XDEV))
>> return ERR_PTR(-EXDEV);
>> }
>> /* rare case of legitimate dget_parent()... */
>> - parent = dget_parent(nd->path.dentry);
>> + return dget_parent(path->dentry);
>
> I have some confusion with this patch when crossing mount boundary.
>
> In d_path.c, we have
>
> static int __prepend_path(const struct dentry *dentry, const struct mount *mnt,
> const struct path *root, struct prepend_buffer *p)
> {
> while (dentry != root->dentry || &mnt->mnt != root->mnt) {
> const struct dentry *parent = READ_ONCE(dentry->d_parent);
>
> if (dentry == mnt->mnt.mnt_root) {
> struct mount *m = READ_ONCE(mnt->mnt_parent);
> struct mnt_namespace *mnt_ns;
>
> if (likely(mnt != m)) {
> dentry = READ_ONCE(mnt->mnt_mountpoint);
> mnt = m;
> continue;
> }
> /* Global root */
> mnt_ns = READ_ONCE(mnt->mnt_ns);
> /* open-coded is_mounted() to use local mnt_ns */
> if (!IS_ERR_OR_NULL(mnt_ns) && !is_anon_ns(mnt_ns))
> return 1; // absolute root
> else
> return 2; // detached or not attached yet
> }
>
> if (unlikely(dentry == parent))
> /* Escaped? */
> return 3;
>
> prefetch(parent);
> if (!prepend_name(p, &dentry->d_name))
> break;
> dentry = parent;
> }
> return 0;
> }
>
> At the mount boundary and not at root mount, the code has
> dentry = READ_ONCE(mnt->mnt_mountpoint);
> mnt = m; /* 'mnt' will be parent mount */
> continue;
>
> After that, we have
> const struct dentry *parent = READ_ONCE(dentry->d_parent);
> if (dentry == mnt->mnt.mnt_root) {
> /* assume this is false */
> }
> ...
> prefetch(parent);
> if (!prepend_name(p, &dentry->d_name))
> break;
> dentry = parent;
>
> So the prepend_name(p, &dentry->d_name) is actually from mnt->mnt_mountpoint.
I am not quite following the question. In the code below:
if (dentry == mnt->mnt.mnt_root) {
struct mount *m = READ_ONCE(mnt->mnt_parent);
struct mnt_namespace *mnt_ns;
if (likely(mnt != m)) {
dentry = READ_ONCE(mnt->mnt_mountpoint);
mnt = m;
continue;
/* We either continue, here */
}
/* Global root */
mnt_ns = READ_ONCE(mnt->mnt_ns);
/* open-coded is_mounted() to use local mnt_ns */
if (!IS_ERR_OR_NULL(mnt_ns) && !is_anon_ns(mnt_ns))
return 1; // absolute root
else
return 2; // detached or not attached yet
/* Or return here */
}
So we will not hit prepend_name(). Does this answer the
question?
>
> In your above code, maybe we should return path->dentry in the below if statement?
>
> if (unlikely(path->dentry == path->mnt->mnt_root)) {
> struct path new_path;
>
> if (!choose_mountpoint(real_mount(path->mnt),
> root, &new_path))
> goto in_root;
> path_put(path);
> *path = new_path;
> if (unlikely(flags & LOOKUP_NO_XDEV))
> return ERR_PTR(-EXDEV);
> + return path->dentry;
> }
> /* rare case of legitimate dget_parent()... */
> return dget_parent(path->dentry);
>
> Also, could you add some selftests cross mount points? This will
> have more coverages with __path_walk_parent().
Yeah, I will try to add more tests in the next revision.
Thanks,
Song
Powered by blists - more mailing lists