* [PATCH bpf-next 1/5] fs/kernel_read_file,selinux: Add BPF_LOADER constant
2026-08-13 0:26 [PATCH bpf-next 0/5] bpf: Introduce LOADER_LOAD_FD Thiébaud Weksteen
@ 2026-08-13 0:26 ` Thiébaud Weksteen
2026-08-13 0:36 ` sashiko-bot
2026-08-13 1:25 ` bot+bpf-ci
2026-08-13 0:26 ` [PATCH bpf-next 2/5] bpf: Introduce BPF_LOADER_LOAD_FD command Thiébaud Weksteen
` (3 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Thiébaud Weksteen @ 2026-08-13 0:26 UTC (permalink / raw)
To: Paul Moore, Stephen Smalley, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Jeffrey Vander Stoep
Cc: Thiébaud Weksteen, Ondrej Mosnacek, Eric Suen,
Blaise Boscaccy, Sid Nayyar, Neill Kapron, Eric Biggers,
Greg Kroah-Hartman, KP Singh, bpf, selinux, linux-kernel
Add a new constant for kernel_read_file when loading a BPF loader. Add
the matching SELinux policy for that constant.
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
---
include/linux/kernel_read_file.h | 1 +
security/selinux/hooks.c | 12 ++++++++++--
security/selinux/include/classmap.h | 2 +-
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index d613a7b4dd35..fbcaf41c1b73 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -15,6 +15,7 @@
id(POLICY, security-policy) \
id(X509_CERTIFICATE, x509-certificate) \
id(MODULE_COMPRESSED, kernel-module-compressed) \
+ id(BPF_LOADER, bpf-loader) \
id(MAX_ID, )
#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 18dd28b2bb13..f197cf476190 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4411,7 +4411,7 @@ static int selinux_kernel_read_file(struct file *file,
{
int rc = 0;
- BUILD_BUG_ON_MSG(READING_MAX_ID > 8,
+ BUILD_BUG_ON_MSG(READING_MAX_ID > 9,
"New kernel_read_file_id introduced; update SELinux!");
switch (id) {
@@ -4437,6 +4437,10 @@ static int selinux_kernel_read_file(struct file *file,
rc = selinux_kernel_load_from_file(file,
SYSTEM__X509_CERTIFICATE_LOAD);
break;
+ case READING_BPF_LOADER:
+ rc = selinux_kernel_load_from_file(file,
+ SYSTEM__BPF_LOAD);
+ break;
default:
break;
}
@@ -4448,7 +4452,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
{
int rc = 0;
- BUILD_BUG_ON_MSG(LOADING_MAX_ID > 8,
+ BUILD_BUG_ON_MSG(LOADING_MAX_ID > 9,
"New kernel_load_data_id introduced; update SELinux!");
switch (id) {
@@ -4474,6 +4478,10 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
rc = selinux_kernel_load_from_file(NULL,
SYSTEM__X509_CERTIFICATE_LOAD);
break;
+ case LOADING_BPF_LOADER:
+ rc = selinux_kernel_load_from_file(NULL,
+ SYSTEM__BPF_LOAD);
+ break;
default:
break;
}
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 90cb61b16425..453522ca87df 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -65,7 +65,7 @@ const struct security_class_mapping secclass_map[] = {
{ "ipc_info", "syslog_read", "syslog_mod", "syslog_console",
"module_request", "module_load", "firmware_load",
"kexec_image_load", "kexec_initramfs_load", "policy_load",
- "x509_certificate_load", NULL } },
+ "x509_certificate_load", "bpf_load", NULL } },
{ "capability", { COMMON_CAP_PERMS, NULL } },
{ "filesystem",
{ "mount", "remount", "unmount", "getattr", "relabelfrom",
--
2.55.0.691.gc56d675ccc-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next 1/5] fs/kernel_read_file,selinux: Add BPF_LOADER constant
2026-08-13 0:26 ` [PATCH bpf-next 1/5] fs/kernel_read_file,selinux: Add BPF_LOADER constant Thiébaud Weksteen
@ 2026-08-13 0:36 ` sashiko-bot
2026-08-13 1:25 ` bot+bpf-ci
1 sibling, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-13 0:36 UTC (permalink / raw)
To: Thiébaud Weksteen; +Cc: bpf, selinux
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added enum variants `READING_BPF_LOADER` and `LOADING_BPF_LOADER` are not handled in the switch statements of the IPE (Integrity Policy Enforcement) LSM hooks. This will cause a kernel warning and default to an invalid policy operation.
--
--- Patch [1]: [PATCH bpf-next 1/5] fs/kernel_read_file,selinux: Add BPF_LOADER constant ---
commit 2c2b2df398b783718511b45e4ea8aa7251362d31
Author: Thiébaud Weksteen <tweek@google.com>
fs/kernel_read_file,selinux: Add BPF_LOADER constant
This commit adds a new BPF_LOADER constant for kernel_read_file_id and
kernel_load_data_id. It also updates the SELinux policy to handle the new
loading constants.
> diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
> index d613a7b4dd35a..fbcaf41c1b734 100644
> --- a/include/linux/kernel_read_file.h
> +++ b/include/linux/kernel_read_file.h
> @@ -15,6 +15,7 @@
> id(POLICY, security-policy) \
> id(X509_CERTIFICATE, x509-certificate) \
> id(MODULE_COMPRESSED, kernel-module-compressed) \
> + id(BPF_LOADER, bpf-loader) \
[Severity: High]
Should the Integrity Policy Enforcement (IPE) LSM hooks be updated to handle
these new constants as well?
If READING_BPF_LOADER is passed to ipe_kernel_read_file(), it will fall
through to the default case since there is no matching rule setup:
security/ipe/hooks.c:ipe_kernel_read_file() {
...
switch (id) {
...
default:
op = IPE_OP_INVALID;
WARN(1, "no rule setup for kernel_read_file enum %d", id);
}
}
This will trigger a kernel warning and default to an invalid policy operation,
which may incorrectly deny the BPF loader. The same unhandled switch case
exists for LOADING_BPF_LOADER in ipe_kernel_load_data().
> id(MAX_ID, )
>
> #define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813002618.3755631-1-tweek@google.com?part=1
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next 1/5] fs/kernel_read_file,selinux: Add BPF_LOADER constant
2026-08-13 0:26 ` [PATCH bpf-next 1/5] fs/kernel_read_file,selinux: Add BPF_LOADER constant Thiébaud Weksteen
2026-08-13 0:36 ` sashiko-bot
@ 2026-08-13 1:25 ` bot+bpf-ci
1 sibling, 0 replies; 15+ messages in thread
From: bot+bpf-ci @ 2026-08-13 1:25 UTC (permalink / raw)
To: tweek, paul, stephen.smalley.work, ast, daniel, andrii, jeffv
Cc: tweek, omosnace, ericsu, bboscaccy, sidnayyar, nkapron, ebiggers,
gregkh, kpsingh, bpf, selinux, linux-kernel, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2284 bytes --]
> diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
> index d613a7b4dd35a..fbcaf41c1b734 100644
> --- a/include/linux/kernel_read_file.h
> +++ b/include/linux/kernel_read_file.h
> @@ -15,6 +15,7 @@
> id(POLICY, security-policy) \
> id(X509_CERTIFICATE, x509-certificate) \
> id(MODULE_COMPRESSED, kernel-module-compressed) \
> + id(BPF_LOADER, bpf-loader) \
> id(MAX_ID, )
[ ... ]
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 90cb61b164256..453522ca87df8 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -65,7 +65,7 @@ const struct security_class_mapping secclass_map[] = {
> { "ipc_info", "syslog_read", "syslog_mod", "syslog_console",
> "module_request", "module_load", "firmware_load",
> "kexec_image_load", "kexec_initramfs_load", "policy_load",
> - "x509_certificate_load", NULL } },
> + "x509_certificate_load", "bpf_load", NULL } },
> { "capability", { COMMON_CAP_PERMS, NULL } },
Does "bpf_load" match the pattern the rest of class 'system' follows?
Every other kernel_read_file-derived permission follows the pattern
<lowercase_id>_load:
id(MODULE, ...) -> "module_load"
id(FIRMWARE, ...) -> "firmware_load"
id(KEXEC_IMAGE, ...) -> "kexec_image_load"
id(KEXEC_INITRAMFS, ...) -> "kexec_initramfs_load"
id(POLICY, ...) -> "policy_load"
id(X509_CERTIFICATE, ...) -> "x509_certificate_load"
id(BPF_LOADER, ...) -> "bpf_load" <-- drops "loader"
Following that pattern, wouldn't "bpf_loader_load" read more consistently
here - especially since the 'bpf' class separately gains "loader_load_fd"
later in the series (commit f0d1e716f1dc8)?
The shortened name is also ambiguous: the 'bpf' class already has
"prog_load" for loading BPF programs, and this permission specifically
gates kernel_read_file() operations on loader ELF files, not BPF in
general. SELinux permission names become policy ABI once merged.
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31655295294
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next 2/5] bpf: Introduce BPF_LOADER_LOAD_FD command
2026-08-13 0:26 [PATCH bpf-next 0/5] bpf: Introduce LOADER_LOAD_FD Thiébaud Weksteen
2026-08-13 0:26 ` [PATCH bpf-next 1/5] fs/kernel_read_file,selinux: Add BPF_LOADER constant Thiébaud Weksteen
@ 2026-08-13 0:26 ` Thiébaud Weksteen
2026-08-13 0:42 ` sashiko-bot
2026-08-13 1:40 ` bot+bpf-ci
2026-08-13 0:26 ` [PATCH bpf-next 3/5] selinux: use kernel sid in security_bpf_* Thiébaud Weksteen
` (2 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Thiébaud Weksteen @ 2026-08-13 0:26 UTC (permalink / raw)
To: Paul Moore, Stephen Smalley, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Jeffrey Vander Stoep
Cc: Thiébaud Weksteen, Ondrej Mosnacek, Eric Suen,
Blaise Boscaccy, Sid Nayyar, Neill Kapron, Eric Biggers,
Greg Kroah-Hartman, KP Singh, bpf, selinux, linux-kernel
Introduce the BPF_LOADER_LOAD_FD command to allow loading and executing
loader BPF programs directly from an ELF file.
This command implements the equivalent of bpf_load_and_run within the
kernel. More specifically, it implements the four steps:
1. Create an array map
2. Populate the array with the loader data
3. Load the loader, using BPF_PROG_LOAD
4. Execute the loader using BPF_PROG_TEST_RUN
BPF_LOADER_LOAD_FD takes 3 arguments, a file descriptor to an open ELF
which contains the instructions, data and license of the loader; a
context and its size which are passed to BPF_PROG_TEST_RUN.
The kernel validates the ELF file and extracts three sections:
1. __loader.prog: Contains the loader instructions.
2. __loader.map: Contains the loader data.
3. license
The caller is expected to use libbpf's light skeleton generator. The
loader program is responsible for managing any maps or programs included
in the original BPF object. CO-RE and BTF are explicitly not supported
by the kernel here; they are handled by the loader directly.
BPF_LOADER_LOAD_FD returns the updated context to userspace. The kernel
treats this context as an opaque blob passed to BPF_PROG_TEST_RUN. For
the userspace loader, this context typically contains the file
descriptors of the newly created maps and programs.
ELF validation borrows logic from the kernel module ELF validation
in kernel/module/main.c.
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
---
include/uapi/linux/bpf.h | 7 +
kernel/bpf/syscall.c | 341 ++++++++++++++++++++++++++++++++-
tools/include/uapi/linux/bpf.h | 7 +
3 files changed, 348 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ffd96e8b920b..05b070a489fc 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -993,6 +993,7 @@ enum bpf_cmd {
BPF_TOKEN_CREATE,
BPF_PROG_STREAM_READ_BY_FD,
BPF_PROG_ASSOC_STRUCT_OPS,
+ BPF_LOADER_LOAD_FD,
__MAX_BPF_CMD,
BPF_COMMON_ATTRS = 1 << 16, /* Indicate carrying syscall common attrs. */
};
@@ -1950,6 +1951,12 @@ union bpf_attr {
__u32 flags;
} prog_assoc_struct_ops;
+ struct { /* struct used by BPF_LOADER_LOAD_FD command */
+ __u32 loader_fd;
+ __aligned_u64 ctx;
+ __u32 ctx_size;
+ } load_fd;
+
} __attribute__((aligned(8)));
/* The description below is an attempt at providing documentation to eBPF
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 8d111da88655..d79cd63f9f7c 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -41,6 +41,7 @@
#include <linux/overflow.h>
#include <linux/cookie.h>
#include <linux/btf_ids.h>
+#include <linux/kernel_read_file.h>
#include <net/netfilter/nf_bpf_link.h>
#include <net/netkit.h>
@@ -6291,6 +6292,336 @@ static int prog_assoc_struct_ops(union bpf_attr *attr)
return ret;
}
+#define BPF_LOADER_PROG_SEC "__loader.prog"
+#define BPF_LOADER_MAP_SEC "__loader.map"
+#define BPF_LOADER_LICENSE_SEC "license"
+#define BPF_LOADER_MAX_SIZE (8U << 20) /* 8MB */
+
+struct elf_info {
+ Elf64_Ehdr *hdr;
+ unsigned long len;
+ Elf64_Shdr *sechdrs;
+ char *secstrings;
+};
+
+static int bpf_validate_section_offset(const struct elf_info *info, Elf64_Shdr *shdr)
+{
+ unsigned long long secend;
+
+ /*
+ * Check for both overflow and offset/size being
+ * too large.
+ */
+ secend = shdr->sh_offset + shdr->sh_size;
+ if (secend < shdr->sh_offset || secend > info->len)
+ return -ENOEXEC;
+
+ return 0;
+}
+
+static int bpf_elf_validity_ehdr(const struct elf_info *info)
+{
+ if (info->len < sizeof(*(info->hdr))) {
+ pr_err("Invalid ELF header len %lu\n", info->len);
+ return -ENOEXEC;
+ }
+ if (memcmp(info->hdr->e_ident, ELFMAG, SELFMAG) != 0) {
+ pr_err("Invalid ELF header magic: != %s\n", ELFMAG);
+ return -ENOEXEC;
+ }
+ if (info->hdr->e_ident[EI_CLASS] != ELFCLASS64) {
+ pr_err("Only 64-bit ELF is supported\n");
+ return -ENOEXEC;
+ }
+ if (info->hdr->e_type != ET_REL) {
+ pr_err("Invalid ELF header type: %u != %u\n",
+ info->hdr->e_type, ET_REL);
+ return -ENOEXEC;
+ }
+ if (info->hdr->e_machine != EM_BPF) {
+ pr_err("Invalid ELF machine type: %u != %u\n",
+ info->hdr->e_machine, EM_BPF);
+ return -ENOEXEC;
+ }
+ return 0;
+}
+
+static int bpf_elf_validity_cache_sechdrs(struct elf_info *info)
+{
+ Elf64_Shdr *sechdrs;
+ Elf64_Shdr *shdr;
+ int i;
+ int err;
+
+ err = bpf_elf_validity_ehdr(info);
+ if (err < 0)
+ return err;
+
+ if (info->hdr->e_shentsize != sizeof(Elf64_Shdr)) {
+ pr_err("Invalid ELF section header size\n");
+ return -ENOEXEC;
+ }
+
+ /*
+ * e_shnum is 16 bits, and sizeof(Elf64_Shdr) is
+ * known and small. So e_shnum * sizeof(Elf64_Shdr)
+ * will not overflow unsigned long on any platform.
+ */
+ if (info->hdr->e_shoff >= info->len
+ || (info->hdr->e_shnum * sizeof(Elf64_Shdr) >
+ info->len - info->hdr->e_shoff)) {
+ pr_err("Invalid ELF section header overflow\n");
+ return -ENOEXEC;
+ }
+
+ sechdrs = (void *)info->hdr + info->hdr->e_shoff;
+
+ /*
+ * The code assumes that section 0 has a length of zero and
+ * an addr of zero, so check for it.
+ */
+ if (sechdrs[0].sh_type != SHT_NULL
+ || sechdrs[0].sh_size != 0
+ || sechdrs[0].sh_addr != 0) {
+ pr_err("ELF Spec violation: section 0 type(%d)!=SH_NULL or non-zero len or addr\n",
+ sechdrs[0].sh_type);
+ return -ENOEXEC;
+ }
+
+ /* Validate contents are inbounds */
+ for (i = 1; i < info->hdr->e_shnum; i++) {
+ shdr = &sechdrs[i];
+ switch (shdr->sh_type) {
+ case SHT_NULL:
+ case SHT_NOBITS:
+ /* No contents, offset/size don't mean anything */
+ continue;
+ default:
+ err = bpf_validate_section_offset(info, shdr);
+ if (err < 0) {
+ pr_err("Invalid ELF section in BPF loader (section %u type %u)\n",
+ i, shdr->sh_type);
+ return err;
+ }
+ }
+ }
+
+ info->sechdrs = sechdrs;
+
+ return 0;
+}
+
+static int bpf_elf_validity_cache_secstrings(struct elf_info *info)
+{
+ Elf64_Shdr *strhdr, *shdr;
+ char *secstrings;
+ int i;
+
+ /*
+ * Verify if the section name table index is valid.
+ */
+ if (info->hdr->e_shstrndx == SHN_UNDEF
+ || info->hdr->e_shstrndx >= info->hdr->e_shnum) {
+ pr_err("Invalid ELF section name index: %d || e_shstrndx (%d) >= e_shnum (%d)\n",
+ info->hdr->e_shstrndx, info->hdr->e_shstrndx,
+ info->hdr->e_shnum);
+ return -ENOEXEC;
+ }
+
+ strhdr = &info->sechdrs[info->hdr->e_shstrndx];
+
+ if (strhdr->sh_type != SHT_STRTAB) {
+ pr_err("Invalid ELF section name table type: %u\n", strhdr->sh_type);
+ return -ENOEXEC;
+ }
+
+ /*
+ * The section name table must be NUL-terminated, as required
+ * by the spec. This makes strcmp and pr_* calls that access
+ * strings in the section safe.
+ */
+ secstrings = (void *)info->hdr + strhdr->sh_offset;
+ if (strhdr->sh_size == 0) {
+ pr_err("empty section name table\n");
+ return -ENOEXEC;
+ }
+ if (secstrings[strhdr->sh_size - 1] != '\0') {
+ pr_err("ELF Spec violation: section name table isn't null terminated\n");
+ return -ENOEXEC;
+ }
+
+ for (i = 0; i < info->hdr->e_shnum; i++) {
+ shdr = &info->sechdrs[i];
+ /* SHT_NULL means sh_name has an undefined value */
+ if (shdr->sh_type == SHT_NULL)
+ continue;
+ if (shdr->sh_name >= strhdr->sh_size) {
+ pr_err("Invalid ELF section name in BPF loader (section %u type %u)\n",
+ i, shdr->sh_type);
+ return -ENOEXEC;
+ }
+ }
+
+ info->secstrings = secstrings;
+ return 0;
+}
+
+static int find_elf_section(const struct elf_info *info,
+ const char *sect_name, void **sect, int *sect_sz)
+{
+ Elf64_Shdr *shdr;
+
+ for (int i = 1; i < info->hdr->e_shnum; i++) {
+ shdr = &info->sechdrs[i];
+ if (shdr->sh_type == SHT_NULL || shdr->sh_type == SHT_NOBITS)
+ continue;
+ if (strcmp(sect_name, info->secstrings + shdr->sh_name) == 0) {
+ *sect = (void *)info->hdr + shdr->sh_offset;
+ *sect_sz = shdr->sh_size;
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
+/* To shut up -Wmissing-prototypes.
+ * This function is used by the kernel light skeleton
+ * to load bpf programs when modules are loaded or during kernel boot.
+ * See tools/lib/bpf/skel_internal.h
+ */
+int kern_sys_bpf(int cmd, union bpf_attr *attr, unsigned int size);
+
+#define BPF_LOADER_LOAD_FD_LAST_FIELD load_fd.ctx_size
+
+static int loader_load_fd(union bpf_attr *attr)
+{
+ void *buf = NULL, *insns = NULL, *data = NULL, *license = NULL;
+ void *kctx = NULL;
+ int len, err = 0;
+ int insns_sz = 0, data_sz = 0, license_sz = 0;
+ int map_fd, prog_fd;
+ size_t ctx_sz;
+ union bpf_attr sattr = { 0 };
+ unsigned int zero = 0;
+
+ if (!capable(CAP_BPF))
+ return -EPERM;
+
+ if (CHECK_ATTR(BPF_LOADER_LOAD_FD))
+ return -EINVAL;
+
+ if (attr->load_fd.ctx_size > U16_MAX)
+ return -EINVAL;
+
+ CLASS(fd, f)(attr->load_fd.loader_fd);
+ if (fd_empty(f))
+ return -EINVAL;
+
+ len = kernel_read_file(fd_file(f), 0, &buf, BPF_LOADER_MAX_SIZE, NULL,
+ READING_BPF_LOADER);
+ if (len < 0) {
+ err = len;
+ goto out;
+ }
+
+ struct elf_info elf_info = {
+ .hdr = (Elf64_Ehdr *) buf,
+ .len = len,
+ };
+
+ err = bpf_elf_validity_cache_sechdrs(&elf_info);
+ if (err)
+ goto out_free_buf;
+
+ err = bpf_elf_validity_cache_secstrings(&elf_info);
+ if (err)
+ goto out_free_buf;
+
+ err = find_elf_section(&elf_info, BPF_LOADER_PROG_SEC, &insns, &insns_sz);
+ if (err)
+ goto out_free_buf;
+
+ err = find_elf_section(&elf_info, BPF_LOADER_MAP_SEC, &data, &data_sz);
+ if (err)
+ goto out_free_buf;
+
+ err = find_elf_section(&elf_info, BPF_LOADER_LICENSE_SEC, &license, &license_sz);
+ if (err)
+ goto out_free_buf;
+
+ if (license_sz == 0 || ((char *)license)[license_sz - 1] != '\0') {
+ pr_err("ELF Spec violation: license section isn't null terminated\n");
+ err = -ENOEXEC;
+ goto out_free_buf;
+ }
+
+ memset(&sattr, 0, sizeof(sattr));
+ sattr.map_type = BPF_MAP_TYPE_ARRAY;
+ sattr.key_size = sizeof(unsigned int);
+ sattr.value_size = data_sz;
+ sattr.max_entries = 1;
+ map_fd = kern_sys_bpf(BPF_MAP_CREATE, &sattr, sizeof(sattr));
+ if (map_fd < 0) {
+ err = map_fd;
+ goto out_free_buf;
+ }
+
+ memset(&sattr, 0, sizeof(sattr));
+ sattr.map_fd = map_fd;
+ sattr.key = (unsigned long) &zero;
+ sattr.value = (unsigned long) data;
+ err = kern_sys_bpf(BPF_MAP_UPDATE_ELEM, &sattr, sizeof(sattr));
+ if (err < 0)
+ goto close_map_err;
+
+ memset(&sattr, 0, sizeof(sattr));
+ sattr.prog_type = BPF_PROG_TYPE_SYSCALL;
+ sattr.license = (unsigned long) license;
+ sattr.insns = (unsigned long) insns;
+ sattr.insn_cnt = insns_sz / sizeof(struct bpf_insn);
+ sattr.fd_array = (unsigned long) &map_fd;
+ sattr.prog_flags = BPF_F_SLEEPABLE;
+ strscpy(sattr.prog_name, BPF_LOADER_PROG_SEC, sizeof(BPF_LOADER_PROG_SEC));
+ prog_fd = kern_sys_bpf(BPF_PROG_LOAD, &sattr, sizeof(sattr));
+ if (prog_fd < 0) {
+ err = prog_fd;
+ goto close_map_err;
+ }
+
+ memset(&sattr, 0, sizeof(sattr));
+ ctx_sz = attr->load_fd.ctx_size;
+ kctx = kzalloc(ctx_sz, GFP_KERNEL);
+ if (kctx == NULL) {
+ err = -ENOMEM;
+ goto close_prog_err;
+ }
+ sattr.test.prog_fd = prog_fd;
+ sattr.test.ctx_in = (unsigned long) kctx;
+ sattr.test.ctx_size_in = ctx_sz;
+ err = kern_sys_bpf(BPF_PROG_TEST_RUN, &sattr, sizeof(sattr));
+ if (err < 0)
+ goto free_ctx;
+ err = sattr.test.retval;
+ if (err < 0)
+ goto free_ctx;
+
+ if (copy_to_user((void *) attr->load_fd.ctx, kctx, ctx_sz) != 0)
+ err = -EFAULT;
+
+free_ctx:
+ kfree(kctx);
+close_prog_err:
+ close_fd(prog_fd);
+close_map_err:
+ close_fd(map_fd);
+out_free_buf:
+ vfree(buf);
+out:
+ return err;
+}
+
+
static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
bpfptr_t uattr_common, unsigned int size_common)
{
@@ -6463,6 +6794,9 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
case BPF_PROG_ASSOC_STRUCT_OPS:
err = prog_assoc_struct_ops(&attr);
break;
+ case BPF_LOADER_LOAD_FD:
+ err = loader_load_fd(&attr);
+ break;
default:
err = -EINVAL;
break;
@@ -6508,13 +6842,6 @@ BPF_CALL_3(bpf_sys_bpf, int, cmd, union bpf_attr *, attr, u32, attr_size)
}
-/* To shut up -Wmissing-prototypes.
- * This function is used by the kernel light skeleton
- * to load bpf programs when modules are loaded or during kernel boot.
- * See tools/lib/bpf/skel_internal.h
- */
-int kern_sys_bpf(int cmd, union bpf_attr *attr, unsigned int size);
-
int kern_sys_bpf(int cmd, union bpf_attr *attr, unsigned int size)
{
struct bpf_prog * __maybe_unused prog;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index ffd96e8b920b..470e3b575497 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -993,6 +993,7 @@ enum bpf_cmd {
BPF_TOKEN_CREATE,
BPF_PROG_STREAM_READ_BY_FD,
BPF_PROG_ASSOC_STRUCT_OPS,
+ BPF_LOADER_LOAD_FD,
__MAX_BPF_CMD,
BPF_COMMON_ATTRS = 1 << 16, /* Indicate carrying syscall common attrs. */
};
@@ -1950,6 +1951,12 @@ union bpf_attr {
__u32 flags;
} prog_assoc_struct_ops;
+ struct { /* struct used by BPF_LOADER_LOAD_FD command */
+ __u32 loader_fd;
+ __aligned_u64 ctx;
+ __u32 ctx_size;
+ } load_fd;
+
} __attribute__((aligned(8)));
/* The description below is an attempt at providing documentation to eBPF
--
2.55.0.691.gc56d675ccc-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next 2/5] bpf: Introduce BPF_LOADER_LOAD_FD command
2026-08-13 0:26 ` [PATCH bpf-next 2/5] bpf: Introduce BPF_LOADER_LOAD_FD command Thiébaud Weksteen
@ 2026-08-13 0:42 ` sashiko-bot
2026-08-13 1:40 ` bot+bpf-ci
1 sibling, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-13 0:42 UTC (permalink / raw)
To: Thiébaud Weksteen; +Cc: selinux, bpf
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Information Leak / Out-of-Bounds Read in BPF verifier via loader_load_fd
- [High] Out-of-bounds Read / Denial of Service in ELF header validation
- [Medium] File Descriptor Hijacking Race Condition (TOCTOU)
- [Medium] API Misuse / Broken functionality on non-JIT configurations
- [Low] Direct cast of `__aligned_u64` to `void *` for `copy_to_user`
--
--- Patch [2]: [PATCH bpf-next 2/5] bpf: Introduce BPF_LOADER_LOAD_FD command ---
commit f9a928bc7f89cffc836753c26236211ed8039c51
Author: Thiébaud Weksteen <tweek@google.com>
bpf: Introduce BPF_LOADER_LOAD_FD command
This commit introduces the BPF_LOADER_LOAD_FD command to allow loading
and executing loader BPF programs directly from an ELF file.
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 8d111da886553..d79cd63f9f7cd 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
[ ... ]
> @@ -6291,6 +6292,336 @@ static int prog_assoc_struct_ops(union bpf_attr *attr)
[ ... ]
> +static int bpf_elf_validity_cache_sechdrs(struct elf_info *info)
> +{
> + Elf64_Shdr *sechdrs;
> + Elf64_Shdr *shdr;
> + int i;
> + int err;
> +
> + err = bpf_elf_validity_ehdr(info);
> + if (err < 0)
> + return err;
> +
> + if (info->hdr->e_shentsize != sizeof(Elf64_Shdr)) {
> + pr_err("Invalid ELF section header size\n");
> + return -ENOEXEC;
> + }
> +
> + /*
> + * e_shnum is 16 bits, and sizeof(Elf64_Shdr) is
> + * known and small. So e_shnum * sizeof(Elf64_Shdr)
> + * will not overflow unsigned long on any platform.
> + */
> + if (info->hdr->e_shoff >= info->len
> + || (info->hdr->e_shnum * sizeof(Elf64_Shdr) >
> + info->len - info->hdr->e_shoff)) {
> + pr_err("Invalid ELF section header overflow\n");
> + return -ENOEXEC;
> + }
> +
> + sechdrs = (void *)info->hdr + info->hdr->e_shoff;
> +
> + /*
> + * The code assumes that section 0 has a length of zero and
> + * an addr of zero, so check for it.
> + */
> + if (sechdrs[0].sh_type != SHT_NULL
[Severity: High]
Does this bounds check properly handle the case where info->hdr->e_shnum
is 0?
If an attacker provides an ELF file with e_shnum set to 0 and e_shoff set
near the end of the file (e.g., info->len - 1), the size check evaluates
0 > 1, which is false. The code will then proceed to access sechdrs[0]
unconditionally.
Since info->hdr is backed by a vmalloc buffer, could this read past the
buffer boundary into the unmapped guard page and cause a panic?
[ ... ]
> +static int loader_load_fd(union bpf_attr *attr)
> +{
[ ... ]
> + memset(&sattr, 0, sizeof(sattr));
> + sattr.prog_type = BPF_PROG_TYPE_SYSCALL;
> + sattr.license = (unsigned long) license;
> + sattr.insns = (unsigned long) insns;
> + sattr.insn_cnt = insns_sz / sizeof(struct bpf_insn);
> + sattr.fd_array = (unsigned long) &map_fd;
[Severity: Critical]
Is it safe to pass a kernel stack pointer (&map_fd) via sattr.fd_array
while leaving fd_array_cnt implicitly as 0?
When fd_array_cnt is 0, the BPF verifier falls back to the legacy sparse
array resolution path in fd_array_get_map_idx_sparse(). In this path, the
verifier extracts an immediate value from a BPF_PSEUDO_MAP_FD instruction
and uses it as an index.
Since fd_array is treated as a kernel pointer, copy_from_bpfptr_offset()
expands directly to a memory copy:
memcpy(&fd, &map_fd + idx, sizeof(int));
Because the index is attacker-controlled and lacks bounds checking, could
this allow an unprivileged user to reliably read arbitrary memory from the
kernel stack?
> + sattr.prog_flags = BPF_F_SLEEPABLE;
> + strscpy(sattr.prog_name, BPF_LOADER_PROG_SEC, sizeof(BPF_LOADER_PROG_SEC));
> + prog_fd = kern_sys_bpf(BPF_PROG_LOAD, &sattr, sizeof(sattr));
> + if (prog_fd < 0) {
> + err = prog_fd;
> + goto close_map_err;
> + }
> +
> + memset(&sattr, 0, sizeof(sattr));
> + ctx_sz = attr->load_fd.ctx_size;
> + kctx = kzalloc(ctx_sz, GFP_KERNEL);
> + if (kctx == NULL) {
> + err = -ENOMEM;
> + goto close_prog_err;
> + }
> + sattr.test.prog_fd = prog_fd;
> + sattr.test.ctx_in = (unsigned long) kctx;
> + sattr.test.ctx_size_in = ctx_sz;
> + err = kern_sys_bpf(BPF_PROG_TEST_RUN, &sattr, sizeof(sattr));
[Severity: Medium]
Does kern_sys_bpf() support BPF_PROG_TEST_RUN when CONFIG_BPF_JIT is
disabled?
Looking at kern_sys_bpf(), it only intercepts BPF_PROG_TEST_RUN when
CONFIG_BPF_JIT is enabled. Without JIT, it falls through to ____bpf_sys_bpf()
which explicitly rejects BPF_PROG_TEST_RUN and returns -EINVAL.
Could this cause the BPF_LOADER_LOAD_FD command to be completely broken on
non-JIT kernel configurations?
> + if (err < 0)
> + goto free_ctx;
> + err = sattr.test.retval;
> + if (err < 0)
> + goto free_ctx;
> +
> + if (copy_to_user((void *) attr->load_fd.ctx, kctx, ctx_sz) != 0)
[Severity: Low]
Is it correct to directly cast __aligned_u64 to a void pointer here?
Directly casting attr->load_fd.ctx to (void *) bypasses Sparse address space
annotations for __user memory. On 32-bit architectures, this can also cause
pointer truncation warnings.
Should this use the u64_to_user_ptr() macro instead?
> + err = -EFAULT;
> +
> +free_ctx:
> + kfree(kctx);
> +close_prog_err:
> + close_fd(prog_fd);
> +close_map_err:
> + close_fd(map_fd);
[Severity: Medium]
Could unconditionally closing these file descriptors introduce a race
condition?
The kern_sys_bpf() function allocates file descriptors and installs them,
making them temporarily visible in the calling process's file descriptor
table.
If a concurrent thread blocks copy_to_user() (for instance, by passing a
userfaultfd-backed memory region for attr->load_fd.ctx), it could close
these exposed file descriptors and open new ones.
When the page fault resolves, would loader_load_fd() resume and inadvertently
close the newly opened files that now occupy those descriptor slots?
> +out_free_buf:
> + vfree(buf);
> +out:
> + return err;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813002618.3755631-1-tweek@google.com?part=2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next 2/5] bpf: Introduce BPF_LOADER_LOAD_FD command
2026-08-13 0:26 ` [PATCH bpf-next 2/5] bpf: Introduce BPF_LOADER_LOAD_FD command Thiébaud Weksteen
2026-08-13 0:42 ` sashiko-bot
@ 2026-08-13 1:40 ` bot+bpf-ci
1 sibling, 0 replies; 15+ messages in thread
From: bot+bpf-ci @ 2026-08-13 1:40 UTC (permalink / raw)
To: tweek, paul, stephen.smalley.work, ast, daniel, andrii, jeffv
Cc: tweek, omosnace, ericsu, bboscaccy, sidnayyar, nkapron, ebiggers,
gregkh, kpsingh, bpf, selinux, linux-kernel, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 8487 bytes --]
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index ffd96e8b920bd..05b070a489fc2 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -993,6 +993,7 @@ enum bpf_cmd {
> BPF_TOKEN_CREATE,
> BPF_PROG_STREAM_READ_BY_FD,
> BPF_PROG_ASSOC_STRUCT_OPS,
> + BPF_LOADER_LOAD_FD,
> __MAX_BPF_CMD,
> BPF_COMMON_ATTRS = 1 << 16, /* Indicate carrying syscall common attrs. */
> };
> @@ -1950,6 +1951,12 @@ union bpf_attr {
> __u32 flags;
> } prog_assoc_struct_ops;
>
> + struct { /* struct used by BPF_LOADER_LOAD_FD command */
> + __u32 loader_fd;
> + __aligned_u64 ctx;
> + __u32 ctx_size;
> + } load_fd;
^^^^
The new load_fd member has an implicit 4-byte padding hole between
loader_fd and ctx that CHECK_ATTR() cannot validate. The compiler inserts
padding at offset 4..7 because __aligned_u64 enforces 8-byte alignment,
but CHECK_ATTR uses BPF_LOADER_LOAD_FD_LAST_FIELD = load_fd.ctx_size,
which only checks from offset 20 onward. This means bytes 4..7 can contain
arbitrary garbage, and a future kernel can never add a field there (like a
flags word) because existing kernels accept non-zero values in that region.
Every other similar member in this union explicitly reserves the hole with
the __u32 :32; bitfield idiom. Could the same pattern be used here?
Also, the three field declarations use spaces after the type name instead
of tabs, while every other member of union bpf_attr uses tabs. Should these
match the surrounding style?
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 8d111da886553..d79cd63f9f7cd 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -41,6 +41,7 @@
> #include <linux/overflow.h>
> #include <linux/cookie.h>
> #include <linux/btf_ids.h>
> +#include <linux/kernel_read_file.h>
The patch adds substantial ELF-parsing code that uses Elf64_Ehdr,
Elf64_Shdr, ELFMAG, SELFMAG, EI_CLASS, ELFCLASS64, ET_REL, EM_BPF,
SHT_NULL, SHT_NOBITS, SHT_STRTAB, and SHN_UNDEF, but does not add
#include <linux/elf.h>. The code compiles only because linux/bpf.h
transitively includes linux/module.h, which includes linux/elf.h.
Should this file directly include what it uses?
[ ... ]
> +static int bpf_elf_validity_ehdr(const struct elf_info *info)
> +{
> + if (info->len < sizeof(*(info->hdr))) {
> + pr_err("Invalid ELF header len %lu\n", info->len);
> + return -ENOEXEC;
> + }
> + if (memcmp(info->hdr->e_ident, ELFMAG, SELFMAG) != 0) {
> + pr_err("Invalid ELF header magic: != %s\n", ELFMAG);
> + return -ENOEXEC;
> + }
> + if (info->hdr->e_ident[EI_CLASS] != ELFCLASS64) {
> + pr_err("Only 64-bit ELF is supported\n");
> + return -ENOEXEC;
> + }
> + if (info->hdr->e_type != ET_REL) {
> + pr_err("Invalid ELF header type: %u != %u\n",
> + info->hdr->e_type, ET_REL);
> + return -ENOEXEC;
> + }
> + if (info->hdr->e_machine != EM_BPF) {
> + pr_err("Invalid ELF machine type: %u != %u\n",
> + info->hdr->e_machine, EM_BPF);
> + return -ENOEXEC;
> + }
> + return 0;
> +}
The validation path adds 15 unratelimited pr_err() calls on a path that
userspace can drive in a loop with only CAP_BPF. A trivial loop over
bpf(BPF_LOADER_LOAD_FD, ...) with a malformed ELF floods dmesg and evicts
unrelated log data. Additionally, no pr_fmt is defined in this file, so
the messages appear in dmesg with no subsystem prefix, and several strings
are byte-identical to the module loader's messages, making it impossible to
tell which subsystem emitted the line.
Should these use pr_err_ratelimited(), or could they be routed through the
verifier log mechanism that the caller already receives?
[ ... ]
> +static int loader_load_fd(union bpf_attr *attr)
> +{
> + void *buf = NULL, *insns = NULL, *data = NULL, *license = NULL;
> + void *kctx = NULL;
> + int len, err = 0;
> + int insns_sz = 0, data_sz = 0, license_sz = 0;
> + int map_fd, prog_fd;
> + size_t ctx_sz;
> + union bpf_attr sattr = { 0 };
> + unsigned int zero = 0;
> +
> + if (!capable(CAP_BPF))
> + return -EPERM;
This uses capable(CAP_BPF) where the rest of the bpf() syscall uses
bpf_capable(). bpf_capable() is defined as capable(CAP_BPF) ||
capable(CAP_SYS_ADMIN), so a CAP_SYS_ADMIN-only caller is rejected here
with -EPERM, even though they can already issue BPF_MAP_CREATE,
BPF_PROG_LOAD, and BPF_PROG_TEST_RUN by hand. This is also the only
capable(CAP_BPF) call in this file; every other privileged command uses
bpf_capable() or capable(CAP_SYS_ADMIN).
Should this use bpf_capable() instead?
[ ... ]
> + CLASS(fd, f)(attr->load_fd.loader_fd);
> + if (fd_empty(f))
> + return -EINVAL;
[ ... ]
> +free_ctx:
> + kfree(kctx);
> +close_prog_err:
> + close_fd(prog_fd);
> +close_map_err:
> + close_fd(map_fd);
> +out_free_buf:
> + vfree(buf);
> +out:
> + return err;
A subsystem pattern flags this as potentially concerning: the function
mixes scope-based cleanup (CLASS(fd, f) at the top) with goto-based cleanup
(the five-label ladder at the bottom). include/linux/cleanup.h expects
that a routine converts all resources to scope-based cleanup or none.
I traced every exit path and found no functional defect: the CLASS(fd, f)
destructor is at function scope and runs on all goto paths and all three
early returns; the goto ladder correctly skips close_fd(map_fd) and
close_fd(prog_fd) for descriptors that were not yet installed; and
vfree(NULL) / kfree(NULL) are no-ops. Is this interleaving intentional, or
would it be worth converting the remaining resources to scope-based cleanup
to keep the style consistent?
[ ... ]
> + memset(&sattr, 0, sizeof(sattr));
> + sattr.prog_type = BPF_PROG_TYPE_SYSCALL;
> + sattr.license = (unsigned long) license;
> + sattr.insns = (unsigned long) insns;
> + sattr.insn_cnt = insns_sz / sizeof(struct bpf_insn);
> + sattr.fd_array = (unsigned long) &map_fd;
> + sattr.prog_flags = BPF_F_SLEEPABLE;
> + strscpy(sattr.prog_name, BPF_LOADER_PROG_SEC, sizeof(BPF_LOADER_PROG_SEC));
^^^^^^^^^^^^^^^^^^^^^^^
The strscpy() size argument is the size of the source string, not the
destination. sattr.prog_name is BPF_OBJ_NAME_LEN (16) bytes, and
sizeof("__loader.prog") is 14, so it fits today, but the bound no longer
describes the destination. Would sizeof(sattr.prog_name) or the two-
argument form be more appropriate here?
[ ... ]
> + memset(&sattr, 0, sizeof(sattr));
> + ctx_sz = attr->load_fd.ctx_size;
> + kctx = kzalloc(ctx_sz, GFP_KERNEL);
The commit message states "BPF_LOADER_LOAD_FD takes 3 arguments, ... a
context and its size which are passed to BPF_PROG_TEST_RUN" and "returns
the updated context," but the implementation kzalloc()s a fresh zero-filled
buffer and never copies the caller's context in. Only the copy_to_user() at
the end happens. This means attr->load_fd.ctx is write-only, and the input
half of struct bpf_loader_ctx is silently discarded: log_level, log_size,
log_buf, and flags are all forced to zero.
A practical consequence is that the loader program can never emit a
verifier log, so a failing BPF_LOADER_LOAD_FD gives userspace an errno and
nothing else. Is this behavior intentional, or should the caller's ctx be
copied in?
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index ffd96e8b920bd..470e3b575497d 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -993,6 +993,7 @@ enum bpf_cmd {
> BPF_TOKEN_CREATE,
> BPF_PROG_STREAM_READ_BY_FD,
> BPF_PROG_ASSOC_STRUCT_OPS,
> + BPF_LOADER_LOAD_FD,
> __MAX_BPF_CMD,
> BPF_COMMON_ATTRS = 1 << 16, /* Indicate carrying syscall common attrs. */
> };
> @@ -1950,6 +1951,12 @@ union bpf_attr {
> __u32 flags;
> } prog_assoc_struct_ops;
>
> + struct { /* struct used by BPF_LOADER_LOAD_FD command */
> + __u32 loader_fd;
> + __aligned_u64 ctx;
> + __u32 ctx_size;
> + } load_fd;
The tools/ copy of the header uses tabs for field alignment, while the
kernel copy in include/uapi/linux/bpf.h uses spaces. This makes diff -u
include/uapi/linux/bpf.h tools/include/uapi/linux/bpf.h report three
differing lines after this commit. The tools/ copy is expected to be a
byte-for-byte mirror of the kernel header. Should both use tabs?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31655295294
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next 3/5] selinux: use kernel sid in security_bpf_*
2026-08-13 0:26 [PATCH bpf-next 0/5] bpf: Introduce LOADER_LOAD_FD Thiébaud Weksteen
2026-08-13 0:26 ` [PATCH bpf-next 1/5] fs/kernel_read_file,selinux: Add BPF_LOADER constant Thiébaud Weksteen
2026-08-13 0:26 ` [PATCH bpf-next 2/5] bpf: Introduce BPF_LOADER_LOAD_FD command Thiébaud Weksteen
@ 2026-08-13 0:26 ` Thiébaud Weksteen
2026-08-13 0:40 ` sashiko-bot
2026-08-13 1:25 ` bot+bpf-ci
2026-08-13 0:26 ` [PATCH bpf-next 4/5] selinux: Add BPF_LOADER_LOAD_FD syscall permission Thiébaud Weksteen
2026-08-13 0:26 ` [PATCH bpf-next 5/5] selftests/bpf: add loader_load_fd tests Thiébaud Weksteen
4 siblings, 2 replies; 15+ messages in thread
From: Thiébaud Weksteen @ 2026-08-13 0:26 UTC (permalink / raw)
To: Paul Moore, Stephen Smalley, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Jeffrey Vander Stoep
Cc: Thiébaud Weksteen, Ondrej Mosnacek, Eric Suen,
Blaise Boscaccy, Sid Nayyar, Neill Kapron, Eric Biggers,
Greg Kroah-Hartman, KP Singh, bpf, selinux, linux-kernel
The security_bpf hooks provides a boolean to indicate if the call is
coming from within the kernel or not. If true, use the kernel SID
instead of relying on the current process SID.
For the token-aware functions, the kernel sid is used to decide on the
access, but the caller remains owner of the object (program or map).
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
---
security/selinux/hooks.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f197cf476190..e7c5993f6954 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7181,7 +7181,7 @@ static int selinux_ib_alloc_security(void *ib_sec)
static int selinux_bpf(int cmd, union bpf_attr *attr,
unsigned int size, bool kernel)
{
- u32 sid = current_sid();
+ u32 sid = kernel ? SECINITSID_KERNEL : current_sid();
int ret;
if (selinux_policycap_bpf_token_perms())
@@ -7296,7 +7296,7 @@ static int selinux_bpf_map_create(struct bpf_map *map, union bpf_attr *attr,
bpfsec->sid = current_sid();
if (!token)
- ssid = bpfsec->sid;
+ ssid = kernel ? SECINITSID_KERNEL : bpfsec->sid;
else
ssid = selinux_bpffs_creator_sid(attr->map_token_fd);
@@ -7314,7 +7314,7 @@ static int selinux_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr,
bpfsec->sid = current_sid();
if (!token)
- ssid = bpfsec->sid;
+ ssid = kernel ? SECINITSID_KERNEL : bpfsec->sid;
else
ssid = selinux_bpffs_creator_sid(attr->prog_token_fd);
--
2.55.0.691.gc56d675ccc-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next 3/5] selinux: use kernel sid in security_bpf_*
2026-08-13 0:26 ` [PATCH bpf-next 3/5] selinux: use kernel sid in security_bpf_* Thiébaud Weksteen
@ 2026-08-13 0:40 ` sashiko-bot
2026-08-13 1:25 ` bot+bpf-ci
1 sibling, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-13 0:40 UTC (permalink / raw)
To: Thiébaud Weksteen; +Cc: selinux, bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] A Time-of-Check to Time-of-Use (TOCTOU) race condition in SELinux BPF hooks allows unprivileged users to steal the `creator_sid` of a highly-privileged file (e.g., on procfs), bypassing SELinux policy restrictions for BPF operations.
--
--- Patch [3]: [PATCH bpf-next 3/5] selinux: use kernel sid in security_bpf_* ---
commit 74fdd38ce08ed06b1a82fae50df139cbd161c51e
Author: Thiébaud Weksteen <tweek@google.com>
selinux: use kernel sid in security_bpf_*
The security_bpf hooks provides a boolean to indicate if the call is
coming from within the kernel or not. If true, use the kernel SID
instead of relying on the current process SID. For the token-aware
functions, the kernel sid is used to decide on the access, but the caller
remains owner of the object (program or map).
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f197cf476190d..e7c5993f69545 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
[ ... ]
> @@ -7296,7 +7296,7 @@ static int selinux_bpf_map_create(struct bpf_map *map, union bpf_attr *attr,
> bpfsec->sid = current_sid();
>
> if (!token)
> - ssid = bpfsec->sid;
> + ssid = kernel ? SECINITSID_KERNEL : bpfsec->sid;
> else
> ssid = selinux_bpffs_creator_sid(attr->map_token_fd);
[Severity: Critical]
This is a pre-existing issue, but does using selinux_bpffs_creator_sid()
with the raw attr->map_token_fd here expose a Time-of-Check to Time-of-Use
(TOCTOU) race condition?
Since this performs a secondary file descriptor lookup, could a concurrent
thread replace the file descriptor (e.g., via dup3()) with a highly
privileged file before this SELinux hook runs?
If the core kernel has already safely resolved the token object, would it be
safer to use the token's security struct directly instead of performing a
completely new FD table lookup?
> @@ -7314,7 +7314,7 @@ static int selinux_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr,
> bpfsec->sid = current_sid();
>
> if (!token)
> - ssid = bpfsec->sid;
> + ssid = kernel ? SECINITSID_KERNEL : bpfsec->sid;
> else
> ssid = selinux_bpffs_creator_sid(attr->prog_token_fd);
[Severity: Critical]
This is a pre-existing issue, but similar to selinux_bpf_map_create() above,
does this secondary lookup of attr->prog_token_fd allow an attacker to
swap the file descriptor mid-syscall and bypass SELinux restrictions?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813002618.3755631-1-tweek@google.com?part=3
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next 3/5] selinux: use kernel sid in security_bpf_*
2026-08-13 0:26 ` [PATCH bpf-next 3/5] selinux: use kernel sid in security_bpf_* Thiébaud Weksteen
2026-08-13 0:40 ` sashiko-bot
@ 2026-08-13 1:25 ` bot+bpf-ci
1 sibling, 0 replies; 15+ messages in thread
From: bot+bpf-ci @ 2026-08-13 1:25 UTC (permalink / raw)
To: tweek, paul, stephen.smalley.work, ast, daniel, andrii, jeffv
Cc: tweek, omosnace, ericsu, bboscaccy, sidnayyar, nkapron, ebiggers,
gregkh, kpsingh, bpf, selinux, linux-kernel, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 4954 bytes --]
> The security_bpf hooks provides a boolean to indicate if the call is
> coming from within the kernel or not. If true, use the kernel SID
> instead of relying on the current process SID.
>
> For the token-aware functions, the kernel sid is used to decide on the
> access, but the caller remains owner of the object (program or map).
>
> Signed-off-by: Thiébaud Weksteen <tweek@google.com>
The changelog claims that for the token-aware hooks "the kernel sid is
used to decide on the access". Is that accurate when a token is present?
Looking at selinux_bpf_map_create():
if (!token)
ssid = kernel ? SECINITSID_KERNEL : bpfsec->sid;
else
ssid = selinux_bpffs_creator_sid(attr->map_token_fd);
The same pattern appears in selinux_bpf_prog_load(). When a token is
supplied, the kernel argument has no effect - the SID comes from
selinux_bpffs_creator_sid() regardless. Should the commit message clarify
that the kernel SID applies only when no token is present?
This looks like a bug fix for the kernel parameter support added in
082f1db02c80. Should this include:
Fixes: 082f1db02c80 ("security: Propagate caller information in bpf hooks")
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f197cf476190d..e7c5993f69545 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7181,7 +7181,7 @@ static int selinux_ib_alloc_security(void *ib_sec)
> static int selinux_bpf(int cmd, union bpf_attr *attr,
> unsigned int size, bool kernel)
> {
> - u32 sid = current_sid();
> + u32 sid = kernel ? SECINITSID_KERNEL : current_sid();
> int ret;
>
> if (selinux_policycap_bpf_token_perms())
> @@ -7296,7 +7296,7 @@ static int selinux_bpf_map_create(struct bpf_map *map, union bpf_attr *attr,
> bpfsec->sid = current_sid();
>
> if (!token)
> - ssid = bpfsec->sid;
> + ssid = kernel ? SECINITSID_KERNEL : bpfsec->sid;
> else
> ssid = selinux_bpffs_creator_sid(attr->map_token_fd);
The kernel argument of the security_bpf hooks is not a "trusted kernel
origin" indicator - it is uattr.is_kernel, which only indicates whether
the bpf_attr buffer lives in kernel or user address space. Can
substituting SECINITSID_KERNEL based on that flag change the SELinux
decision for a pre-existing userspace code path?
This path is reachable without BPF_LOADER_LOAD_FD. Looking at
kernel/bpf/syscall.c, syscall_prog_func_proto() exposes bpf_sys_bpf() to
any BPF_PROG_TYPE_SYSCALL program that passes bpf_token_capable(token,
CAP_PERFMON). BPF_CALL_3(bpf_sys_bpf, ...) accepts BPF_MAP_CREATE and
BPF_PROG_LOAD and calls:
return __sys_bpf(cmd, KERNEL_BPFPTR(attr), attr_size,
KERNEL_BPFPTR(NULL), 0);
which triggers:
security_bpf(cmd, &attr, size, uattr.is_kernel); /* true */
security_bpf_map_create(map, attr, token, uattr.is_kernel); /* true */
security_bpf_prog_load(prog, attr, token, uattr.is_kernel); /* true */
Such a program is loaded and executed entirely at userspace request via
BPF_PROG_TEST_RUN. This is the libbpf light-skeleton loader
(tools/lib/bpf/skel_internal.h declares kern_sys_bpf(), used by
bpf_load_and_run()), which predates this series.
For a caller in domain X using a light skeleton:
Before this patch:
selinux_bpf() checks avc_has_perm(X, X, BPF, map_create)
selinux_bpf_map_create() checks avc_has_perm(X, X, BPF, map_create)
After this patch:
selinux_bpf() checks avc_has_perm(kernel_t, kernel_t, BPF,
map_create)
selinux_bpf_map_create() checks avc_has_perm(kernel_t, X, BPF,
map_create)
selinux_bpf_prog_load() checks avc_has_perm(kernel_t, X, BPF,
prog_load)
The rule that used to authorize this (allow X self:bpf map_create) no
longer applies, so existing policies get new AVC denials on a path that
worked before. Conversely, in policies where kernel_t is broadly
privileged, domain X gains map_create and prog_load permissions it was
never granted.
This behavior change is not gated. The immediately preceding behavior
change in the same function is gated on selinux_policycap_bpf_token_perms(),
which is the established SELinux convention for this kind of decision
change. Should this new SID substitution also be gated on a policy
capability?
> @@ -7314,7 +7314,7 @@ static int selinux_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr,
> bpfsec->sid = current_sid();
>
> if (!token)
> - ssid = bpfsec->sid;
> + ssid = kernel ? SECINITSID_KERNEL : bpfsec->sid;
> else
> ssid = selinux_bpffs_creator_sid(attr->prog_token_fd);
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31655295294
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next 4/5] selinux: Add BPF_LOADER_LOAD_FD syscall permission
2026-08-13 0:26 [PATCH bpf-next 0/5] bpf: Introduce LOADER_LOAD_FD Thiébaud Weksteen
` (2 preceding siblings ...)
2026-08-13 0:26 ` [PATCH bpf-next 3/5] selinux: use kernel sid in security_bpf_* Thiébaud Weksteen
@ 2026-08-13 0:26 ` Thiébaud Weksteen
2026-08-13 0:41 ` sashiko-bot
2026-08-13 0:26 ` [PATCH bpf-next 5/5] selftests/bpf: add loader_load_fd tests Thiébaud Weksteen
4 siblings, 1 reply; 15+ messages in thread
From: Thiébaud Weksteen @ 2026-08-13 0:26 UTC (permalink / raw)
To: Paul Moore, Stephen Smalley, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Jeffrey Vander Stoep
Cc: Thiébaud Weksteen, Ondrej Mosnacek, Eric Suen,
Blaise Boscaccy, Sid Nayyar, Neill Kapron, Eric Biggers,
Greg Kroah-Hartman, KP Singh, bpf, selinux, linux-kernel
Add the BPF_LOADER_LOAD_FD permission to gate the bpf syscall command of
the same name.
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
---
security/selinux/hooks.c | 4 ++++
security/selinux/include/classmap.h | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e7c5993f6954..b4ff5ea5306d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7196,6 +7196,10 @@ static int selinux_bpf(int cmd, union bpf_attr *attr,
ret = avc_has_perm(sid, sid, SECCLASS_BPF, BPF__PROG_LOAD,
NULL);
break;
+ case BPF_LOADER_LOAD_FD:
+ ret = avc_has_perm(sid, sid, SECCLASS_BPF, BPF__LOADER_LOAD_FD,
+ NULL);
+ break;
default:
ret = 0;
break;
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 453522ca87df..4c6cc71b233c 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -171,7 +171,7 @@ const struct security_class_mapping secclass_map[] = {
{ "infiniband_endport", { "manage_subnet", NULL } },
{ "bpf",
{ "map_create", "map_read", "map_write", "prog_load", "prog_run",
- "map_create_as", "prog_load_as", NULL } },
+ "map_create_as", "prog_load_as", "loader_load_fd", NULL } },
{ "xdp_socket", { COMMON_SOCK_PERMS, NULL } },
{ "mctp_socket", { COMMON_SOCK_PERMS, NULL } },
{ "perf_event",
--
2.55.0.691.gc56d675ccc-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next 4/5] selinux: Add BPF_LOADER_LOAD_FD syscall permission
2026-08-13 0:26 ` [PATCH bpf-next 4/5] selinux: Add BPF_LOADER_LOAD_FD syscall permission Thiébaud Weksteen
@ 2026-08-13 0:41 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-13 0:41 UTC (permalink / raw)
To: Thiébaud Weksteen; +Cc: bpf, selinux
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The SELinux access control check for BPF_LOADER_LOAD_FD is completely bypassed if the bpf_token_perms policy capability is enabled, leading to Privilege Escalation.
Pre-existing issues:
- [High] BPF_LOADER_LOAD_FD fails unconditionally with -EINVAL if CONFIG_BPF_JIT is disabled.
--
--- Patch [4]: [PATCH bpf-next 4/5] selinux: Add BPF_LOADER_LOAD_FD syscall permission ---
commit aa42284da62cb224bfd501dfd4306e6584b6a8e1
Author: Thiébaud Weksteen <tweek@google.com>
selinux: Add BPF_LOADER_LOAD_FD syscall permission
Add the BPF_LOADER_LOAD_FD permission to gate the bpf syscall command of
the same name.
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e7c5993f69545..b4ff5ea5306df 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7196,6 +7196,10 @@ static int selinux_bpf(int cmd, union bpf_attr *attr,
> ret = avc_has_perm(sid, sid, SECCLASS_BPF, BPF__PROG_LOAD,
> NULL);
> break;
> + case BPF_LOADER_LOAD_FD:
> + ret = avc_has_perm(sid, sid, SECCLASS_BPF, BPF__LOADER_LOAD_FD,
> + NULL);
> + break;
[Severity: High]
Is it possible this check can be completely bypassed if the bpf_token_perms
policy capability is enabled?
Looking at selinux_bpf() earlier in this file, there is an early return:
if (selinux_policycap_bpf_token_perms())
return 0;
Because BPF_LOADER_LOAD_FD doesn't have a fine-grained SELinux hook later on,
this early return skips the access control check.
Furthermore, loader_load_fd() internally calls kern_sys_bpf() which executes
the inner map creations and program loads with is_kernel = true:
return __sys_bpf(cmd, KERNEL_BPFPTR(attr), attr_size, KERNEL_BPFPTR(NULL), 0);
This passes the kernel SID (SECINITSID_KERNEL) to the fine-grained hooks
instead of the user's SID:
ssid = kernel ? SECINITSID_KERNEL : bpfsec->sid;
Will this allow a user with CAP_BPF to bypass their SELinux constraints and
load arbitrary programs as the kernel?
[Severity: High]
This isn't a bug introduced by this patch, but will BPF_LOADER_LOAD_FD fail
unconditionally if CONFIG_BPF_JIT is disabled?
Looking at loader_load_fd() in kernel/bpf/syscall.c, it unconditionally calls:
err = kern_sys_bpf(BPF_PROG_TEST_RUN, &sattr, sizeof(sattr));
However, in kern_sys_bpf(), the BPF_PROG_TEST_RUN case is compiled out when
CONFIG_BPF_JIT is disabled:
#ifdef CONFIG_BPF_JIT
case BPF_PROG_TEST_RUN:
This causes it to fall through to the default handler which returns -EINVAL for
BPF_PROG_TEST_RUN, breaking the feature entirely on non-JIT configurations.
> default:
> ret = 0;
> break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813002618.3755631-1-tweek@google.com?part=4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next 5/5] selftests/bpf: add loader_load_fd tests
2026-08-13 0:26 [PATCH bpf-next 0/5] bpf: Introduce LOADER_LOAD_FD Thiébaud Weksteen
` (3 preceding siblings ...)
2026-08-13 0:26 ` [PATCH bpf-next 4/5] selinux: Add BPF_LOADER_LOAD_FD syscall permission Thiébaud Weksteen
@ 2026-08-13 0:26 ` Thiébaud Weksteen
2026-08-13 0:36 ` sashiko-bot
2026-08-13 1:25 ` bot+bpf-ci
4 siblings, 2 replies; 15+ messages in thread
From: Thiébaud Weksteen @ 2026-08-13 0:26 UTC (permalink / raw)
To: Paul Moore, Stephen Smalley, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Jeffrey Vander Stoep
Cc: Thiébaud Weksteen, Ondrej Mosnacek, Eric Suen,
Blaise Boscaccy, Sid Nayyar, Neill Kapron, Eric Biggers,
Greg Kroah-Hartman, KP Singh, bpf, selinux, linux-kernel
Add user-space selftests for BPF_LOADER_LOAD_FD command. The test ELF is
generated based on the existing light skeleton generator. An awk script
extracts the loader program and map. In the future, it is possible to
add an extra option to `bpftool gen skeleton` to output the ELF file
directly.
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
---
tools/testing/selftests/bpf/Makefile | 8 +-
tools/testing/selftests/bpf/loader_setup.sh | 68 ++++
.../selftests/bpf/prog_tests/loader_load_fd.c | 361 ++++++++++++++++++
.../testing/selftests/bpf/progs/test_loader.c | 21 +
4 files changed, 456 insertions(+), 2 deletions(-)
create mode 100755 tools/testing/selftests/bpf/loader_setup.sh
create mode 100644 tools/testing/selftests/bpf/prog_tests/loader_load_fd.c
create mode 100644 tools/testing/selftests/bpf/progs/test_loader.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index d3655a706482..6d881584abbc 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -525,7 +525,7 @@ LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \
LSKELS := fexit_sleep.c trace_printk.c trace_vprintk.c map_ptr_kern.c \
core_kern.c core_kern_overflow.c test_ringbuf.c \
test_ringbuf_n.c test_ringbuf_map_key.c test_ringbuf_write.c \
- test_ringbuf_overwrite.c
+ test_ringbuf_overwrite.c test_loader.c
LSKELS_SIGNED := fentry_test.c fexit_test.c atomics.c
@@ -577,7 +577,8 @@ TRUNNER_EXTRA_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, \
$$(filter %.c,$(TRUNNER_EXTRA_SOURCES)))
TRUNNER_LIB_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, \
$$(filter %.c,$(TRUNNER_LIB_SOURCES)))
-TRUNNER_EXTRA_HDRS := $$(filter %.h,$(TRUNNER_EXTRA_SOURCES))
+TRUNNER_EXTRA_HDRS := $$(filter %.h,$(TRUNNER_EXTRA_SOURCES)) $$(TRUNNER_OUTPUT)/test_loader_processed.lskel.h
+
TRUNNER_TESTS_HDR := $(TRUNNER_TESTS_DIR)/tests.h
TRUNNER_BPF_SRCS := $$(notdir $$(wildcard $(TRUNNER_BPF_PROGS_DIR)/*.c))
TRUNNER_BPF_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.bpf.o, $$(TRUNNER_BPF_SRCS))
@@ -663,6 +664,9 @@ $(TRUNNER_BPF_LSKELS): %.lskel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
}) && \
rm -f $$(<:.o=.llinked1.o) $$(<:.o=.llinked2.o) $$(<:.o=.llinked3.o)
+$$(TRUNNER_OUTPUT)/test_loader_processed.lskel.h: $$(TRUNNER_OUTPUT)/test_loader.lskel.h loader_setup.sh
+ $$(Q)./loader_setup.sh $$< $$@
+
$(TRUNNER_BPF_LSKELS_SIGNED): %.lskel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
$(Q)$(if $(PERMISSIVE),if [ ! -f $$< ]; then \
$$(RM) $$@; \
diff --git a/tools/testing/selftests/bpf/loader_setup.sh b/tools/testing/selftests/bpf/loader_setup.sh
new file mode 100755
index 000000000000..f86651b0718f
--- /dev/null
+++ b/tools/testing/selftests/bpf/loader_setup.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Google LLC
+#
+# loader_setup.sh - Light skeleton data extraction helper for selftests/bpf
+#
+# This script parses an autogenerated light skeleton header (.lskel.h) produced
+# by bpftool ('bpftool gen skeleton -L'). It extracts the embedded loader data
+# ('opts_data') and loader BPF instructions ('opts_insn') string literals and
+# generates a C header file containing:
+# - loader_test_opts_data[]: Data for the '__loader.map' section.
+# - loader_test_opts_data_sz: Size of opts_data.
+# - loader_test_opts_insn[]: Instructions for the '__loader.prog' section.
+# - loader_test_opts_insn_sz: Size of opts_insn.
+#
+# These extracted sections are used by the 'loader_load_fd' selftest to populate
+# an in-memory ELF object file for kernel 'BPF_LOADER_LOAD_FD' testing.
+#
+# Usage:
+# loader_setup.sh <input_lskel.h> <output_data.h>
+
+INPUT="$1"
+OUTPUT="$2"
+
+if [ -z "$INPUT" ] || [ -z "$OUTPUT" ]; then
+ echo "Usage: $0 <input_lskel.h> <output_data.h>" >&2
+ exit 1
+fi
+
+awk -v input="$INPUT" '
+BEGIN {
+ print "/* Generated from " input " */"
+ in_data = 0
+ in_insn = 0
+}
+/opts_data\[\]/ {
+ in_data = 1
+ sub(/^.*opts_data\[\][^="]*=\s*"/, "")
+ printf "static const char test_loader_opts_data[] __attribute__((__aligned__(8))) = \""
+}
+in_data {
+ if (index($0, "\";") > 0) {
+ sub(/";.*/, "")
+ print $0 "\";"
+ print "static const size_t test_loader_opts_data_sz = " \
+ "sizeof(test_loader_opts_data) - 1;\n"
+ in_data = 0
+ } else {
+ print $0
+ }
+}
+/opts_insn\[\]/ {
+ in_insn = 1
+ sub(/^.*opts_insn\[\][^="]*=\s*"/, "")
+ printf "static const char test_loader_opts_insn[] __attribute__((__aligned__(8))) = \""
+}
+in_insn {
+ if (index($0, "\";") > 0) {
+ sub(/";.*/, "")
+ print $0 "\";"
+ print "static const size_t test_loader_opts_insn_sz = " \
+ "sizeof(test_loader_opts_insn) - 1;"
+ in_insn = 0
+ } else {
+ print $0
+ }
+}
+' "$INPUT" > "$OUTPUT"
diff --git a/tools/testing/selftests/bpf/prog_tests/loader_load_fd.c b/tools/testing/selftests/bpf/prog_tests/loader_load_fd.c
new file mode 100644
index 000000000000..ab971662dd04
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/loader_load_fd.c
@@ -0,0 +1,361 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Google LLC */
+
+#define _GNU_SOURCE
+#include <test_progs.h>
+#include <sys/syscall.h>
+#include <sys/mman.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <libelf.h>
+#include <gelf.h>
+#include <linux/bpf.h>
+
+#include "bpf/skel_internal.h"
+#include "test_loader_processed.lskel.h"
+
+/* Test helper to create an in-memory ELF */
+static int create_loader_elf(const void *insns, size_t insns_sz,
+ const void *map_data, size_t map_data_sz,
+ const char *license, size_t license_sz,
+ bool omit_prog, bool omit_map, bool omit_license)
+{
+ char shstrtab[] = "\0__loader.prog\0__loader.map\0license\0.shstrtab";
+ size_t prog_off = 1;
+ size_t map_off = prog_off + strlen("__loader.prog") + 1;
+ size_t lic_off = map_off + strlen("__loader.map") + 1;
+ size_t shstr_off = lic_off + strlen("license") + 1;
+ size_t shstrtab_sz = sizeof(shstrtab);
+ Elf_Data *shstr_data, *data;
+ Elf64_Shdr *shstr_shdr, *shdr;
+ Elf_Scn *shstr_scn, *scn;
+ Elf64_Ehdr *ehdr;
+ Elf *elf;
+ int fd;
+
+ fd = memfd_create("loader_elf", 0);
+ if (!ASSERT_GE(fd, 0, "memfd_create"))
+ return -1;
+
+ elf_version(EV_CURRENT);
+ elf = elf_begin(fd, ELF_C_WRITE, NULL);
+ if (!ASSERT_OK_PTR(elf, "elf_begin")) {
+ close(fd);
+ return -1;
+ }
+
+ ehdr = elf64_newehdr(elf);
+ if (!ASSERT_OK_PTR(ehdr, "elf64_newehdr"))
+ goto err;
+
+ ehdr->e_ident[EI_MAG0] = ELFMAG0;
+ ehdr->e_ident[EI_MAG1] = ELFMAG1;
+ ehdr->e_ident[EI_MAG2] = ELFMAG2;
+ ehdr->e_ident[EI_MAG3] = ELFMAG3;
+ ehdr->e_ident[EI_CLASS] = ELFCLASS64;
+ ehdr->e_ident[EI_DATA] = ELFDATA2LSB;
+ ehdr->e_ident[EI_VERSION] = EV_CURRENT;
+ ehdr->e_machine = EM_BPF;
+ ehdr->e_type = ET_REL;
+ ehdr->e_version = EV_CURRENT;
+
+ shstr_scn = elf_newscn(elf);
+ shstr_shdr = elf64_getshdr(shstr_scn);
+ shstr_shdr->sh_name = shstr_off;
+ shstr_shdr->sh_type = SHT_STRTAB;
+ shstr_shdr->sh_flags = 0;
+
+ shstr_data = elf_newdata(shstr_scn);
+ shstr_data->d_buf = shstrtab;
+ shstr_data->d_size = shstrtab_sz;
+ shstr_data->d_type = ELF_T_BYTE;
+ shstr_data->d_align = 1;
+
+ ehdr->e_shstrndx = elf_ndxscn(shstr_scn);
+
+ if (!omit_prog && insns && insns_sz > 0) {
+ scn = elf_newscn(elf);
+ shdr = elf64_getshdr(scn);
+ shdr->sh_name = prog_off;
+ shdr->sh_type = SHT_PROGBITS;
+ shdr->sh_flags = SHF_ALLOC | SHF_EXECINSTR;
+
+ data = elf_newdata(scn);
+ data->d_buf = (void *)insns;
+ data->d_size = insns_sz;
+ data->d_type = ELF_T_BYTE;
+ data->d_align = 8;
+ }
+
+ if (!omit_map && map_data && map_data_sz > 0) {
+ scn = elf_newscn(elf);
+ shdr = elf64_getshdr(scn);
+ shdr->sh_name = map_off;
+ shdr->sh_type = SHT_PROGBITS;
+ shdr->sh_flags = SHF_ALLOC;
+
+ data = elf_newdata(scn);
+ data->d_buf = (void *)map_data;
+ data->d_size = map_data_sz;
+ data->d_type = ELF_T_BYTE;
+ data->d_align = 8;
+ }
+
+ if (!omit_license && license && license_sz > 0) {
+ scn = elf_newscn(elf);
+ shdr = elf64_getshdr(scn);
+ shdr->sh_name = lic_off;
+ shdr->sh_type = SHT_PROGBITS;
+ shdr->sh_flags = 0;
+
+ data = elf_newdata(scn);
+ data->d_buf = (void *)license;
+ data->d_size = license_sz;
+ data->d_type = ELF_T_BYTE;
+ data->d_align = 1;
+ }
+
+ if (elf_update(elf, ELF_C_WRITE) < 0)
+ goto err;
+
+ elf_end(elf);
+ lseek(fd, 0, SEEK_SET);
+ return fd;
+
+err:
+ elf_end(elf);
+ close(fd);
+ return -1;
+}
+
+static int sys_bpf_loader_load_fd(int loader_fd, void *ctx, __u32 ctx_size)
+{
+ union bpf_attr attr;
+
+ memset(&attr, 0, sizeof(attr));
+ attr.load_fd.loader_fd = loader_fd;
+ attr.load_fd.ctx = ptr_to_u64(ctx);
+ attr.load_fd.ctx_size = ctx_size;
+
+ return syscall(__NR_bpf, BPF_LOADER_LOAD_FD, &attr, sizeof(attr));
+}
+
+static void test_loader_load_fd_invalid_fd(void)
+{
+ struct bpf_loader_ctx ctx = {};
+ int err;
+
+ err = sys_bpf_loader_load_fd(-1, &ctx, sizeof(ctx));
+ ASSERT_EQ(err, -1, "invalid fd sys_bpf return");
+ ASSERT_EQ(errno, EINVAL, "invalid fd errno");
+}
+
+static void test_loader_load_fd_oversized_ctx(void)
+{
+ struct bpf_insn insns[] = {
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ };
+ char map_data[] = "data";
+ char license[] = "GPL";
+ struct bpf_loader_ctx ctx = {};
+ int elf_fd, err;
+
+ elf_fd = create_loader_elf(insns, sizeof(insns),
+ map_data, sizeof(map_data),
+ license, sizeof(license),
+ false, false, false);
+
+ if (!ASSERT_GE(elf_fd, 0, "create_loader_elf"))
+ return;
+
+ err = sys_bpf_loader_load_fd(elf_fd, &ctx, 65536U);
+ ASSERT_EQ(err, -1, "oversized ctx sys_bpf return");
+ ASSERT_EQ(errno, EINVAL, "oversized ctx errno");
+
+ close(elf_fd);
+}
+
+static void test_loader_load_fd_invalid_elf(void)
+{
+ char garbage[] = "not_an_elf_file_content";
+ struct bpf_loader_ctx ctx = {};
+ int fd, err;
+
+ fd = memfd_create("garbage_file", 0);
+ if (!ASSERT_GE(fd, 0, "memfd_create"))
+ return;
+
+ if (!ASSERT_EQ(write(fd, garbage, sizeof(garbage)), sizeof(garbage), "write garbage")) {
+ close(fd);
+ return;
+ }
+ lseek(fd, 0, SEEK_SET);
+
+ err = sys_bpf_loader_load_fd(fd, &ctx, sizeof(ctx));
+ ASSERT_EQ(err, -1, "invalid elf sys_bpf return");
+ ASSERT_EQ(errno, ENOEXEC, "invalid elf errno");
+
+ close(fd);
+}
+
+static void test_loader_load_fd_missing_prog_sec(void)
+{
+ struct bpf_insn insns[] = {
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ };
+ char map_data[] = "data";
+ char license[] = "GPL";
+ struct bpf_loader_ctx ctx = {};
+ int elf_fd, err;
+
+ elf_fd = create_loader_elf(insns, sizeof(insns),
+ map_data, sizeof(map_data),
+ license, sizeof(license),
+ true, false, false);
+ if (!ASSERT_GE(elf_fd, 0, "create_loader_elf"))
+ return;
+
+ err = sys_bpf_loader_load_fd(elf_fd, &ctx, sizeof(ctx));
+ ASSERT_EQ(err, -1, "missing prog sec sys_bpf return");
+ ASSERT_EQ(errno, EINVAL, "missing prog sec errno");
+
+ close(elf_fd);
+}
+
+static void test_loader_load_fd_missing_map_sec(void)
+{
+ struct bpf_insn insns[] = {
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ };
+ char map_data[] = "data";
+ char license[] = "GPL";
+ struct bpf_loader_ctx ctx = {};
+ int elf_fd, err;
+
+ elf_fd = create_loader_elf(insns, sizeof(insns),
+ map_data, sizeof(map_data),
+ license, sizeof(license),
+ false, true, false);
+ if (!ASSERT_GE(elf_fd, 0, "create_loader_elf"))
+ return;
+
+ err = sys_bpf_loader_load_fd(elf_fd, &ctx, sizeof(ctx));
+ ASSERT_EQ(err, -1, "missing map sec sys_bpf return");
+ ASSERT_EQ(errno, EINVAL, "missing map sec errno");
+
+ close(elf_fd);
+}
+
+static void test_loader_load_fd_missing_license_sec(void)
+{
+ struct bpf_insn insns[] = {
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ };
+ char map_data[] = "data";
+ char license[] = "GPL";
+ struct bpf_loader_ctx ctx = {};
+ int elf_fd, err;
+
+ elf_fd = create_loader_elf(insns, sizeof(insns),
+ map_data, sizeof(map_data),
+ license, sizeof(license),
+ false, false, true);
+ if (!ASSERT_GE(elf_fd, 0, "create_loader_elf"))
+ return;
+
+ err = sys_bpf_loader_load_fd(elf_fd, &ctx, sizeof(ctx));
+ ASSERT_EQ(err, -1, "missing license sec sys_bpf return");
+ ASSERT_EQ(errno, EINVAL, "missing license sec errno");
+
+ close(elf_fd);
+}
+
+static void test_loader_load_fd_loader_failure(void)
+{
+ struct bpf_insn insns[] = {
+ BPF_MOV64_IMM(BPF_REG_0, -EPERM),
+ BPF_EXIT_INSN(),
+ };
+ char map_data[] = "data";
+ char license[] = "GPL";
+ struct bpf_loader_ctx ctx = {};
+ int elf_fd, err;
+
+ elf_fd = create_loader_elf(insns, sizeof(insns),
+ map_data, sizeof(map_data),
+ license, sizeof(license),
+ false, false, false);
+ if (!ASSERT_GE(elf_fd, 0, "create_loader_elf"))
+ return;
+
+ err = sys_bpf_loader_load_fd(elf_fd, &ctx, sizeof(ctx));
+ ASSERT_EQ(err, -1, "loader failure sys_bpf return");
+ ASSERT_EQ(errno, EPERM, "loader failure errno");
+
+ close(elf_fd);
+}
+
+struct test_loader_lskel {
+ struct bpf_loader_ctx ctx;
+ struct {
+ struct bpf_map_desc test_map;
+ } maps;
+ struct {
+ struct bpf_prog_desc probe;
+ } progs;
+ struct {
+ int probe_fd;
+ } links;
+};
+
+static void test_loader_load_fd_lskel(void)
+{
+ struct test_loader_lskel skel = {};
+ int elf_fd, err;
+
+ skel.ctx.sz = (char *)&skel.links - (char *)&skel;
+
+ /* Build fake ELF using extracted opts_insn (__loader.prog) and opts_data (__loader.map) */
+ elf_fd = create_loader_elf(test_loader_opts_insn, test_loader_opts_insn_sz,
+ test_loader_opts_data, test_loader_opts_data_sz,
+ "GPL", sizeof("GPL"),
+ false, false, false);
+ if (!ASSERT_GE(elf_fd, 0, "create_loader_elf_lskel"))
+ return;
+
+ err = sys_bpf_loader_load_fd(elf_fd, &skel.ctx, skel.ctx.sz);
+ ASSERT_OK(err, "sys_bpf_loader_load_fd lskel");
+
+ ASSERT_GT(skel.progs.probe.prog_fd, 0, "test_loader probe prog_fd > 0");
+ ASSERT_GT(skel.maps.test_map.map_fd, 0, "test_loader test_map map_fd > 0");
+
+ if (skel.progs.probe.prog_fd > 0)
+ close(skel.progs.probe.prog_fd);
+ if (skel.maps.test_map.map_fd > 0)
+ close(skel.maps.test_map.map_fd);
+ close(elf_fd);
+}
+
+void test_loader_load_fd(void)
+{
+ if (test__start_subtest("invalid_fd"))
+ test_loader_load_fd_invalid_fd();
+ if (test__start_subtest("oversized_ctx"))
+ test_loader_load_fd_oversized_ctx();
+ if (test__start_subtest("invalid_elf"))
+ test_loader_load_fd_invalid_elf();
+ if (test__start_subtest("missing_prog_sec"))
+ test_loader_load_fd_missing_prog_sec();
+ if (test__start_subtest("missing_map_sec"))
+ test_loader_load_fd_missing_map_sec();
+ if (test__start_subtest("missing_license_sec"))
+ test_loader_load_fd_missing_license_sec();
+ if (test__start_subtest("loader_failure"))
+ test_loader_load_fd_loader_failure();
+ if (test__start_subtest("lskel"))
+ test_loader_load_fd_lskel();
+}
diff --git a/tools/testing/selftests/bpf/progs/test_loader.c b/tools/testing/selftests/bpf/progs/test_loader.c
new file mode 100644
index 000000000000..515e15ca042e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_loader.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARRAY);
+ __uint(max_entries, 4);
+ __type(key, __u32);
+ __type(value, __u64);
+} test_map SEC(".maps");
+
+SEC("socket")
+int probe(void *ctx)
+{
+ __u32 key = 0;
+ __u64 *val = bpf_map_lookup_elem(&test_map, &key);
+
+ return val ? (int)*val : 0;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.55.0.691.gc56d675ccc-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next 5/5] selftests/bpf: add loader_load_fd tests
2026-08-13 0:26 ` [PATCH bpf-next 5/5] selftests/bpf: add loader_load_fd tests Thiébaud Weksteen
@ 2026-08-13 0:36 ` sashiko-bot
2026-08-13 1:25 ` bot+bpf-ci
1 sibling, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-13 0:36 UTC (permalink / raw)
To: Thiébaud Weksteen; +Cc: selinux, bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unchecked pointer returns from libelf functions in `create_loader_elf` will cause a SIGSEGV upon allocation failure.
--
--- Patch [5]: [PATCH bpf-next 5/5] selftests/bpf: add loader_load_fd tests ---
commit b9a0b98c13acbc44d106f86733ac4cdd18481ce7
Author: Thiébaud Weksteen <tweek@google.com>
selftests/bpf: add loader_load_fd tests
This commit adds user-space selftests for the BPF_LOADER_LOAD_FD command.
The test ELF is generated based on the existing light skeleton generator,
using an awk script to extract the loader program and map.
> diff --git a/tools/testing/selftests/bpf/prog_tests/loader_load_fd.c b/tools/testing/selftests/bpf/prog_tests/loader_load_fd.c
> new file mode 100644
> index 0000000000000..ab971662dd047
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/loader_load_fd.c
[ ... ]
> + ehdr->e_ident[EI_VERSION] = EV_CURRENT;
> + ehdr->e_machine = EM_BPF;
> + ehdr->e_type = ET_REL;
> + ehdr->e_version = EV_CURRENT;
> +
> + shstr_scn = elf_newscn(elf);
> + shstr_shdr = elf64_getshdr(shstr_scn);
> + shstr_shdr->sh_name = shstr_off;
> + shstr_shdr->sh_type = SHT_STRTAB;
> + shstr_shdr->sh_flags = 0;
> +
> + shstr_data = elf_newdata(shstr_scn);
> + shstr_data->d_buf = shstrtab;
[Severity: Medium]
Could this crash the test runner if libelf fails to allocate memory?
Earlier in create_loader_elf(), ASSERT_OK_PTR is used to check the return
values of elf_begin() and elf64_newehdr(). However, the pointers returned
by elf_newscn(), elf64_getshdr(), and elf_newdata() are dereferenced here
without prior validation.
If these functions return NULL, the test will abort with a segmentation
fault, preventing subsequent tests from running. This pattern also appears
to be repeated for the .prog, .map, and license sections below.
Would it be better to add ASSERT_OK_PTR checks for these allocations?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813002618.3755631-1-tweek@google.com?part=5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next 5/5] selftests/bpf: add loader_load_fd tests
2026-08-13 0:26 ` [PATCH bpf-next 5/5] selftests/bpf: add loader_load_fd tests Thiébaud Weksteen
2026-08-13 0:36 ` sashiko-bot
@ 2026-08-13 1:25 ` bot+bpf-ci
1 sibling, 0 replies; 15+ messages in thread
From: bot+bpf-ci @ 2026-08-13 1:25 UTC (permalink / raw)
To: tweek, paul, stephen.smalley.work, ast, daniel, andrii, jeffv
Cc: tweek, omosnace, ericsu, bboscaccy, sidnayyar, nkapron, ebiggers,
gregkh, kpsingh, bpf, selinux, linux-kernel, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 14394 bytes --]
> diff --git a/tools/testing/selftests/bpf/loader_setup.sh b/tools/testing/selftests/bpf/loader_setup.sh
> new file mode 100755
> index 0000000000000..f86651b0718f6
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/loader_setup.sh
> @@ -0,0 +1,68 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2026 Google LLC
> +#
> +# loader_setup.sh - Light skeleton data extraction helper for selftests/bpf
> +#
> +# This script parses an autogenerated light skeleton header (.lskel.h) produced
> +# by bpftool ('bpftool gen skeleton -L'). It extracts the embedded loader data
> +# ('opts_data') and loader BPF instructions ('opts_insn') string literals and
> +# generates a C header file containing:
> +# - loader_test_opts_data[]: Data for the '__loader.map' section.
> +# - loader_test_opts_data_sz: Size of opts_data.
> +# - loader_test_opts_insn[]: Instructions for the '__loader.prog' section.
> +# - loader_test_opts_insn_sz: Size of opts_insn.
The comment documents loader_test_opts_data[], loader_test_opts_data_sz,
loader_test_opts_insn[], and loader_test_opts_insn_sz, but the awk body
below emits test_loader_opts_data, test_loader_opts_data_sz,
test_loader_opts_insn and test_loader_opts_insn_sz. Could the comment use
the actual symbol names?
[ ... ]
> +/opts_data\[\]/ {
> + in_data = 1
> + sub(/^.*opts_data\[\][^="]*=\s*"/, "")
> + printf "static const char test_loader_opts_data[] __attribute__((__aligned__(8))) = \""
> +}
Does the awk sub() regex work with mawk or busybox awk? mawk is the
default awk on Debian and Ubuntu, and Documentation/process/changes.rst
lists GNU awk as optional. The \s escape (here and again below in the
opts_insn block) is a GNU awk extension that mawk does not implement.
In mawk, \s matches a literal 's', so the regex =\s*" cannot match
bpftool's output = ", and the sub() fails silently. When the prefix is
not stripped, the emitted line becomes two adjacent string literals
followed by a bare hex-escape token outside any string:
static const char test_loader_opts_data[] = "<TAB>static const char opts_data[] = "\
\x7f\x45\x4c\x46...";"
That is not valid C. This breaks the entire `make -C
tools/testing/selftests/bpf` build because Makefile:580 adds
test_loader_processed.lskel.h to TRUNNER_EXTRA_HDRS for every runner, not
just the loader_load_fd test.
The portable form used across the kernel tree is [[:space:]] - see
scripts/syscallnr.sh, scripts/syscalltbl.sh, scripts/headers_install.sh,
and scripts/tags.sh. Code that genuinely needs GNU awk calls gawk
explicitly.
> +/opts_insn\[\]/ {
> + in_insn = 1
> + sub(/^.*opts_insn\[\][^="]*=\s*"/, "")
> + printf "static const char test_loader_opts_insn[] __attribute__((__aligned__(8))) = \""
> +}
Same \s issue here.
> diff --git a/tools/testing/selftests/bpf/prog_tests/loader_load_fd.c b/tools/testing/selftests/bpf/prog_tests/loader_load_fd.c
> new file mode 100644
> index 0000000000000..ab971662dd047
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/loader_load_fd.c
[ ... ]
> +static int create_loader_elf(const void *insns, size_t insns_sz,
> + const void *map_data, size_t map_data_sz,
> + const char *license, size_t license_sz,
> + bool omit_prog, bool omit_map, bool omit_license)
> +{
[ ... ]
> + ehdr->e_ident[EI_MAG0] = ELFMAG0;
> + ehdr->e_ident[EI_MAG1] = ELFMAG1;
> + ehdr->e_ident[EI_MAG2] = ELFMAG2;
> + ehdr->e_ident[EI_MAG3] = ELFMAG3;
> + ehdr->e_ident[EI_CLASS] = ELFCLASS64;
> + ehdr->e_ident[EI_DATA] = ELFDATA2LSB;
> + ehdr->e_ident[EI_VERSION] = EV_CURRENT;
> + ehdr->e_machine = EM_BPF;
> + ehdr->e_type = ET_REL;
Does hardcoding ELFDATA2LSB work on big-endian hosts?
libelf converts the in-memory structures to the encoding named in
e_ident[EI_DATA] when elf_update() writes the file. With EI_DATA forced
to ELFDATA2LSB, a big-endian host emits byte-swapped headers.
The kernel side parses those headers with native loads and never inspects
e_ident[EI_DATA]:
static int bpf_elf_validity_ehdr(const struct elf_info *info)
{
...
if (info->hdr->e_type != ET_REL) {
pr_err("Invalid ELF header type: %u != %u\n", ...);
return -ENOEXEC;
}
On s390x, ET_REL (1) stored little-endian reads back as 0x0100 == 256, so
loader_load_fd() returns -ENOEXEC. The missing_prog_sec and
missing_map_sec subtests assert EINVAL (not ENOEXEC), so they would fail
against a correct kernel on big-endian hardware.
The section payloads are handed to elf_newdata() as ELF_T_BYTE and copied
in host order, while the headers around them get swapped to LSB. Since the
kernel consumes the file in host byte order, does EI_DATA need to come
from __BYTE_ORDER__? tools/lib/bpf/linker.c and usdt.c use this pattern:
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
const unsigned char host_byteorder = ELFDATA2LSB;
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
const unsigned char host_byteorder = ELFDATA2MSB;
#else
#error "Unknown __BYTE_ORDER__"
#endif
[ ... ]
> +static void test_loader_load_fd_oversized_ctx(void)
> +{
> + struct bpf_insn insns[] = {
> + BPF_MOV64_IMM(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + };
> + char map_data[] = "data";
> + char license[] = "GPL";
> + struct bpf_loader_ctx ctx = {};
> + int elf_fd, err;
> +
> + elf_fd = create_loader_elf(insns, sizeof(insns),
> + map_data, sizeof(map_data),
> + license, sizeof(license),
> + false, false, false);
> +
> + if (!ASSERT_GE(elf_fd, 0, "create_loader_elf"))
> + return;
> +
> + err = sys_bpf_loader_load_fd(elf_fd, &ctx, 65536U);
> + ASSERT_EQ(err, -1, "oversized ctx sys_bpf return");
> + ASSERT_EQ(errno, EINVAL, "oversized ctx errno");
A subsystem pattern flags this as potentially concerning: the subtest
asserts EINVAL for ctx_size == 65536, targeting loader_load_fd()'s guard
if (attr->load_fd.ctx_size > U16_MAX)
return -EINVAL;
but EINVAL is also produced further down the same path. With the guard
removed, the request reaches bpf_prog_test_run_syscall(), which
independently rejects the same value:
if (ctx_size_in < prog->aux->max_ctx_offset ||
ctx_size_in > U16_MAX)
return -EINVAL;
errno is EINVAL either way, so the subtest passes whether or not
loader_load_fd() validates ctx_size. Could the test assert on a value that
only loader_load_fd() rejects, or check the ordering explicitly (e.g. pass
an oversized ctx_size together with an invalid loader_fd and confirm
EINVAL still comes back before any ELF is read)?
Also: ctx is a 24-byte struct on the stack, and ctx_size is 65536.
loader_load_fd() copies ctx_size bytes back on success:
if (copy_to_user((void *) attr->load_fd.ctx, kctx, ctx_sz) != 0)
err = -EFAULT;
The only thing standing between this and a 64 KiB stack overwrite is the
check the subtest is meant to verify. If that check regresses, the
subtest's failure mode is stack corruption in test_progs instead of a
clean assertion failure. Would a 65536-byte heap buffer make the bad-kernel
case report cleanly?
> +static void test_loader_load_fd_invalid_elf(void)
> +{
> + char garbage[] = "not_an_elf_file_content";
> + struct bpf_loader_ctx ctx = {};
> + int fd, err;
> +
> + fd = memfd_create("garbage_file", 0);
> + if (!ASSERT_GE(fd, 0, "memfd_create"))
> + return;
> +
> + if (!ASSERT_EQ(write(fd, garbage, sizeof(garbage)), sizeof(garbage), "write garbage")) {
> + close(fd);
> + return;
> + }
> + lseek(fd, 0, SEEK_SET);
> +
> + err = sys_bpf_loader_load_fd(fd, &ctx, sizeof(ctx));
> + ASSERT_EQ(err, -1, "invalid elf sys_bpf return");
> + ASSERT_EQ(errno, ENOEXEC, "invalid elf errno");
A subsystem pattern flags this as potentially concerning: the subtest
writes 24 bytes and asserts ENOEXEC, but that input never reaches the
magic-number check it appears to target. bpf_elf_validity_ehdr() bails out
on the length test first:
if (info->len < sizeof(*(info->hdr))) {
pr_err("Invalid ELF header len %lu\n", info->len);
return -ENOEXEC;
}
if (memcmp(info->hdr->e_ident, ELFMAG, SELFMAG) != 0) {
pr_err("Invalid ELF header magic: != %s\n", ELFMAG);
return -ENOEXEC;
}
sizeof(Elf64_Ehdr) is 64, so with 24 bytes the memcmp() is unreachable.
Would padding the input to at least 64 bytes make the subtest exercise the
magic check as its name implies?
> +static void test_loader_load_fd_missing_license_sec(void)
> +{
> + struct bpf_insn insns[] = {
> + BPF_MOV64_IMM(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + };
> + char map_data[] = "data";
> + char license[] = "GPL";
> + struct bpf_loader_ctx ctx = {};
> + int elf_fd, err;
> +
> + elf_fd = create_loader_elf(insns, sizeof(insns),
> + map_data, sizeof(map_data),
> + license, sizeof(license),
> + false, false, true);
> + if (!ASSERT_GE(elf_fd, 0, "create_loader_elf"))
> + return;
> +
> + err = sys_bpf_loader_load_fd(elf_fd, &ctx, sizeof(ctx));
> + ASSERT_EQ(err, -1, "missing license sec sys_bpf return");
> + ASSERT_EQ(errno, EINVAL, "missing license sec errno");
A subsystem pattern flags this as potentially concerning: the subtest
asserts errno == EINVAL for an ELF that has no license section. EINVAL is
also what the bpf(2) syscall returns for a command it does not know - in
__sys_bpf() the switch ends with `default: err = -EINVAL;`. So this
assertion is satisfied both by a kernel that correctly rejects the
malformed loader ELF and by a kernel that has never heard of
BPF_LOADER_LOAD_FD at all.
The same is true of invalid_fd, oversized_ctx, missing_prog_sec, and
missing_map_sec, so 5 of the 8 subtests pass unchanged on a kernel without
this patch series. Only invalid_elf (ENOEXEC) and lskel genuinely
discriminate. Could a feature probe up front (e.g. issue one
BPF_LOADER_LOAD_FD call that must not fail with EINVAL-for-unknown-command,
or test__skip() when the well-formed lskel path returns EINVAL) keep
unknown-command kernels from silently passing?
> +static void test_loader_load_fd_loader_failure(void)
> +{
> + struct bpf_insn insns[] = {
> + BPF_MOV64_IMM(BPF_REG_0, -EPERM),
> + BPF_EXIT_INSN(),
> + };
> + char map_data[] = "data";
> + char license[] = "GPL";
> + struct bpf_loader_ctx ctx = {};
> + int elf_fd, err;
> +
> + elf_fd = create_loader_elf(insns, sizeof(insns),
> + map_data, sizeof(map_data),
> + license, sizeof(license),
> + false, false, false);
> + if (!ASSERT_GE(elf_fd, 0, "create_loader_elf"))
> + return;
> +
> + err = sys_bpf_loader_load_fd(elf_fd, &ctx, sizeof(ctx));
> + ASSERT_EQ(err, -1, "loader failure sys_bpf return");
> + ASSERT_EQ(errno, EPERM, "loader failure errno");
A subsystem pattern flags this as potentially concerning: the subtest
builds a loader program that returns -EPERM and asserts errno == EPERM to
prove that loader_load_fd() propagates the loader program's return value.
But EPERM is also the first thing loader_load_fd() can return:
if (!capable(CAP_BPF))
return -EPERM;
That fires before the fd is even resolved. So the assertion is equally
satisfied by a kernel that never ran the loader program at all. Would using
a distinctive errno for the loader's return value (e.g. -E2BIG or
-EBADMSG, something loader_load_fd() itself never produces) make the check
discriminating?
[ ... ]
> +struct test_loader_lskel {
> + struct bpf_loader_ctx ctx;
> + struct {
> + struct bpf_map_desc test_map;
> + } maps;
> + struct {
> + struct bpf_prog_desc probe;
> + } progs;
> + struct {
> + int probe_fd;
> + } links;
> +};
> +
> +static void test_loader_load_fd_lskel(void)
> +{
> + struct test_loader_lskel skel = {};
> + int elf_fd, err;
> +
> + skel.ctx.sz = (char *)&skel.links - (char *)&skel;
> +
> + /* Build fake ELF using extracted opts_insn (__loader.prog) and opts_data (__loader.map) */
> + elf_fd = create_loader_elf(test_loader_opts_insn, test_loader_opts_insn_sz,
> + test_loader_opts_data, test_loader_opts_data_sz,
> + "GPL", sizeof("GPL"),
> + false, false, false);
> + if (!ASSERT_GE(elf_fd, 0, "create_loader_elf_lskel"))
> + return;
> +
> + err = sys_bpf_loader_load_fd(elf_fd, &skel.ctx, skel.ctx.sz);
> + ASSERT_OK(err, "sys_bpf_loader_load_fd lskel");
Does this cover loader context copy-in?
progs/test_loader.c is the only BPF object the lskel subtest exercises,
and it is deliberately minimal: one ARRAY map with a compile-time
max_entries and no global variables, so the generated object has no
.rodata/.data/.bss map.
That makes the subtest blind to the place where the kernel-side
loader_load_fd() diverges from userspace bpf_load_and_run(): the kernel
does not copy the caller's loader context in, it allocates a fresh zeroed
one:
kctx = kzalloc(ctx_sz, GFP_KERNEL);
...
sattr.test.ctx_in = (unsigned long) kctx;
There is no copy_from_user() of attr->load_fd.ctx. The generated loader
program reads two caller-supplied ctx inputs per map via move_ctx2blob():
struct bpf_map_desc::max_entries and struct bpf_map_desc::initial_value.
With a zeroed ctx, initial_value is 0 and the copy is silently skipped, so
any real light skeleton with global/const data would come up with an
all-zero .rodata/.data map instead of its initialised contents. Because
test_loader.c has no such map, the subtest passes either way.
Would adding a global variable to progs/test_loader.c (e.g. const volatile
__u64 expected = 0x1234; plus a check on it) turn this into a covered case?
> diff --git a/tools/testing/selftests/bpf/progs/test_loader.c b/tools/testing/selftests/bpf/progs/test_loader.c
> new file mode 100644
> index 0000000000000..515e15ca042ec
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_loader.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_ARRAY);
> + __uint(max_entries, 4);
> + __type(key, __u32);
> + __type(value, __u64);
> +} test_map SEC(".maps");
> +
> +SEC("socket")
> +int probe(void *ctx)
> +{
> + __u32 key = 0;
> + __u64 *val = bpf_map_lookup_elem(&test_map, &key);
> +
> + return val ? (int)*val : 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31655295294
^ permalink raw reply [flat|nested] 15+ messages in thread