* [PATCH bpf-next] bpf: Mark dentry->d_inode as trusted_or_null
@ 2025-06-13 5:28 Song Liu
2025-06-13 7:24 ` Matt Bobrowski
2025-06-13 18:40 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Song Liu @ 2025-06-13 5:28 UTC (permalink / raw)
To: bpf
Cc: kernel-team, andrii, ast, daniel, martin.lau, eddyz87, kpsingh,
mattbobrowski, Song Liu
LSM hooks such as security_path_mknod() and security_inode_rename() have
access to newly allocated negative dentry, which has NULL d_inode.
Therefore, it is necessary to do the NULL pointer check for d_inode.
Also add selftests that checks the verifier enforces the NULL pointer
check.
Signed-off-by: Song Liu <song@kernel.org>
---
kernel/bpf/verifier.c | 5 ++---
.../selftests/bpf/progs/verifier_vfs_accept.c | 18 ++++++++++++++++++
.../selftests/bpf/progs/verifier_vfs_reject.c | 15 +++++++++++++++
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 14dd836acb13..5c7775cf1259 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7104,8 +7104,7 @@ BTF_TYPE_SAFE_TRUSTED(struct file) {
struct inode *f_inode;
};
-BTF_TYPE_SAFE_TRUSTED(struct dentry) {
- /* no negative dentry-s in places where bpf can see it */
+BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct dentry) {
struct inode *d_inode;
};
@@ -7143,7 +7142,6 @@ static bool type_is_trusted(struct bpf_verifier_env *env,
BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct bpf_iter__task));
BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct linux_binprm));
BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct file));
- BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct dentry));
return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id, "__safe_trusted");
}
@@ -7153,6 +7151,7 @@ static bool type_is_trusted_or_null(struct bpf_verifier_env *env,
const char *field_name, u32 btf_id)
{
BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct socket));
+ BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct dentry));
return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id,
"__safe_trusted_or_null");
diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c b/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
index a7c0a553aa50..3e2d76ee8050 100644
--- a/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
+++ b/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2024 Google LLC. */
#include <vmlinux.h>
+#include <errno.h>
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
@@ -82,4 +83,21 @@ int BPF_PROG(path_d_path_from_file_argument, struct file *file)
return 0;
}
+SEC("lsm.s/inode_rename")
+__success
+int BPF_PROG(inode_rename, struct inode *old_dir, struct dentry *old_dentry,
+ struct inode *new_dir, struct dentry *new_dentry,
+ unsigned int flags)
+{
+ struct inode *inode = new_dentry->d_inode;
+ ino_t ino;
+
+ if (!inode)
+ return 0;
+ ino = inode->i_ino;
+ if (ino == 0)
+ return -EACCES;
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
index d6d3f4fcb24c..4b392c6c8fc4 100644
--- a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
+++ b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2024 Google LLC. */
#include <vmlinux.h>
+#include <errno.h>
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
#include <linux/limits.h>
@@ -158,4 +159,18 @@ int BPF_PROG(path_d_path_kfunc_non_lsm, struct path *path, struct file *f)
return 0;
}
+SEC("lsm.s/inode_rename")
+__failure __msg("invalid mem access 'trusted_ptr_or_null_'")
+int BPF_PROG(inode_rename, struct inode *old_dir, struct dentry *old_dentry,
+ struct inode *new_dir, struct dentry *new_dentry,
+ unsigned int flags)
+{
+ struct inode *inode = new_dentry->d_inode;
+ ino_t ino;
+
+ ino = inode->i_ino;
+ if (ino == 0)
+ return -EACCES;
+ return 0;
+}
char _license[] SEC("license") = "GPL";
--
2.47.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Mark dentry->d_inode as trusted_or_null
2025-06-13 5:28 [PATCH bpf-next] bpf: Mark dentry->d_inode as trusted_or_null Song Liu
@ 2025-06-13 7:24 ` Matt Bobrowski
2025-06-13 18:40 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Matt Bobrowski @ 2025-06-13 7:24 UTC (permalink / raw)
To: Song Liu
Cc: bpf, kernel-team, andrii, ast, daniel, martin.lau, eddyz87,
kpsingh
On Thu, Jun 12, 2025 at 10:28:56PM -0700, Song Liu wrote:
> LSM hooks such as security_path_mknod() and security_inode_rename() have
> access to newly allocated negative dentry, which has NULL d_inode.
> Therefore, it is necessary to do the NULL pointer check for d_inode.
>
> Also add selftests that checks the verifier enforces the NULL pointer
> check.
Thank you for correcting this. Looks OK to me.
Reviewed-by: Matt Bobrowski <mattbobrowski@google.com>
> Signed-off-by: Song Liu <song@kernel.org>
>
> ---
> kernel/bpf/verifier.c | 5 ++---
> .../selftests/bpf/progs/verifier_vfs_accept.c | 18 ++++++++++++++++++
> .../selftests/bpf/progs/verifier_vfs_reject.c | 15 +++++++++++++++
> 3 files changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 14dd836acb13..5c7775cf1259 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -7104,8 +7104,7 @@ BTF_TYPE_SAFE_TRUSTED(struct file) {
> struct inode *f_inode;
> };
>
> -BTF_TYPE_SAFE_TRUSTED(struct dentry) {
> - /* no negative dentry-s in places where bpf can see it */
> +BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct dentry) {
> struct inode *d_inode;
> };
>
> @@ -7143,7 +7142,6 @@ static bool type_is_trusted(struct bpf_verifier_env *env,
> BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct bpf_iter__task));
> BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct linux_binprm));
> BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct file));
> - BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct dentry));
>
> return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id, "__safe_trusted");
> }
> @@ -7153,6 +7151,7 @@ static bool type_is_trusted_or_null(struct bpf_verifier_env *env,
> const char *field_name, u32 btf_id)
> {
> BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct socket));
> + BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct dentry));
>
> return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id,
> "__safe_trusted_or_null");
> diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c b/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
> index a7c0a553aa50..3e2d76ee8050 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
> @@ -2,6 +2,7 @@
> /* Copyright (c) 2024 Google LLC. */
>
> #include <vmlinux.h>
> +#include <errno.h>
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
>
> @@ -82,4 +83,21 @@ int BPF_PROG(path_d_path_from_file_argument, struct file *file)
> return 0;
> }
>
> +SEC("lsm.s/inode_rename")
> +__success
> +int BPF_PROG(inode_rename, struct inode *old_dir, struct dentry *old_dentry,
> + struct inode *new_dir, struct dentry *new_dentry,
> + unsigned int flags)
> +{
> + struct inode *inode = new_dentry->d_inode;
> + ino_t ino;
> +
> + if (!inode)
> + return 0;
> + ino = inode->i_ino;
> + if (ino == 0)
> + return -EACCES;
> + return 0;
> +}
> +
> char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
> index d6d3f4fcb24c..4b392c6c8fc4 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
> @@ -2,6 +2,7 @@
> /* Copyright (c) 2024 Google LLC. */
>
> #include <vmlinux.h>
> +#include <errno.h>
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
> #include <linux/limits.h>
> @@ -158,4 +159,18 @@ int BPF_PROG(path_d_path_kfunc_non_lsm, struct path *path, struct file *f)
> return 0;
> }
>
> +SEC("lsm.s/inode_rename")
> +__failure __msg("invalid mem access 'trusted_ptr_or_null_'")
> +int BPF_PROG(inode_rename, struct inode *old_dir, struct dentry *old_dentry,
> + struct inode *new_dir, struct dentry *new_dentry,
> + unsigned int flags)
> +{
> + struct inode *inode = new_dentry->d_inode;
> + ino_t ino;
> +
> + ino = inode->i_ino;
> + if (ino == 0)
> + return -EACCES;
> + return 0;
> +}
> char _license[] SEC("license") = "GPL";
> --
> 2.47.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Mark dentry->d_inode as trusted_or_null
2025-06-13 5:28 [PATCH bpf-next] bpf: Mark dentry->d_inode as trusted_or_null Song Liu
2025-06-13 7:24 ` Matt Bobrowski
@ 2025-06-13 18:40 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-06-13 18:40 UTC (permalink / raw)
To: Song Liu
Cc: bpf, kernel-team, andrii, ast, daniel, martin.lau, eddyz87,
kpsingh, mattbobrowski
Hello:
This patch was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Thu, 12 Jun 2025 22:28:56 -0700 you wrote:
> LSM hooks such as security_path_mknod() and security_inode_rename() have
> access to newly allocated negative dentry, which has NULL d_inode.
> Therefore, it is necessary to do the NULL pointer check for d_inode.
>
> Also add selftests that checks the verifier enforces the NULL pointer
> check.
>
> [...]
Here is the summary with links:
- [bpf-next] bpf: Mark dentry->d_inode as trusted_or_null
https://git.kernel.org/bpf/bpf/c/d60d09eadb7c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-13 18:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-13 5:28 [PATCH bpf-next] bpf: Mark dentry->d_inode as trusted_or_null Song Liu
2025-06-13 7:24 ` Matt Bobrowski
2025-06-13 18:40 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox