[PATCH bpf-next 0/5] bpf: Follow-up fixes for BPF syscall common attributes

[PATCH bpf-next 0/5] bpf: Follow-up fixes for BPF syscall common attributes

[Leon Hwang]

Address sashiko reviews for BPF syscall common attributes series.

1. The tailing padding bytes in struct bpf_common_attr should be
   checked. [1]
2. There was a concurrent regression in syscall.c::map_create(). [2]
3. OPTS_VALID() was missing to validate the nested struct bpf_log_opts
   in libbpf. [3]
4. The token_fd should be -1 to avoid a valid token fd in test. [4]

A test is added to verify the fix #1.

The fix #2 is hard to be verified by test. So, I decide not to add a test
for it to avoid over-engineering.

Decide not to add a test for fix #3 to avoid over-engineering, as the
fix looks really simple.

Links:
[1] https://lore.kernel.org/bpf/20260513224823.6494FC19425@smtp.kernel.org/
[2] https://lore.kernel.org/bpf/20260512233658.CEED7C2BCB0@smtp.kernel.org/
[3] https://lore.kernel.org/bpf/20260512235629.C5CABC2BCB0@smtp.kernel.org/
[4] https://lore.kernel.org/bpf/20260513003358.55836C2BCB0@smtp.kernel.org/

Leon Hwang (5):
  bpf: Check tail zero of bpf_common_attr using offsetofend
  bpf: Fix concurrent regression in map_create()
  libbpf: Add OPTS_VALID() for log_opts in bpf_map_create
  selftests/bpf: Use -1 as token_fd in map create failure test
  selftests/bpf: Add test to verify checking padding bytes for BPF
    syscall common attributes

 kernel/bpf/syscall.c                          | 19 +++++++++----
 tools/lib/bpf/bpf.c                           |  3 ++
 .../selftests/bpf/prog_tests/map_init.c       | 28 ++++++++++++++++++-
 3 files changed, 44 insertions(+), 6 deletions(-)

-- 
2.54.0

[PATCH bpf-next 1/5] bpf: Check tail zero of bpf_common_attr using offsetofend

[Leon Hwang]

Because of the 8-byte alignment, the compiler will pad struct
bpf_common_attr to 24 bytes. That said, sizeof(attr_common) is 24 instead
of 20.

When check tail zero using sizeof(attr_common) in
bpf_check_uarg_tail_zero(), there will be 4 bytes that won't be checked.

To also check the padding 4 bytes, replace sizeof(attr_common) with
offsetofend(struct bpf_common_attr, log_true_size).

Fixes: f28771c0691b ("bpf: Extend BPF syscall with common attributes support")
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 kernel/bpf/syscall.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6600e126fbfb..83de8fb9b9aa 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -6278,7 +6278,9 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
 
 	memset(&attr_common, 0, sizeof(attr_common));
 	if (cmd & BPF_COMMON_ATTRS) {
-		err = bpf_check_uarg_tail_zero(uattr_common, sizeof(attr_common), size_common);
+		err = bpf_check_uarg_tail_zero(uattr_common,
+					       offsetofend(struct bpf_common_attr, log_true_size),
+					       size_common);
 		if (err)
 			return err;
 
-- 
2.54.0

[PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create()

[Leon Hwang]

Because there is time gap between bpf_map_new_fd() and close_fd(), a
concurrent thread is able to close the new fd and opens a new, unrelated
file with the exact same fd number. Thereafter, this close_fd() might
inadvertently close the unrelated file.

To avoid such regression, drop close_fd() and override err when failed to
create map and failed to finalize the log.

In other word, when succeed in creating map but fail to finalize log,
users will get the map fd instead of the finalization error.

Fixes: 49f9b2b2a18c ("bpf: Add syscall common attributes support for map_create")
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 kernel/bpf/syscall.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 83de8fb9b9aa..322865a88b3a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1647,11 +1647,18 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_at
 
 	/* preserve original error even if log finalization is successful */
 	ret = bpf_log_attr_finalize(&attr_log, log);
-	if (ret) {
-		if (err >= 0)
-			close_fd(err);
+	if (ret && err < 0)
+		/*
+		 * Failed to finalize the log.
+		 * Should not close_fd(err) here. Since the bpf_map_new_fd()
+		 * has published the map fd, if a concurrent thread closes the
+		 * fd, then opens new, unrelated file that receives the exact
+		 * same fd number, close_fd(err) might inadvertently close the
+		 * unrelated file.
+		 * As a trade-off, override the err only when failed to finalize
+		 * the log and failed to create map.
+		 */
 		err = ret;
-	}
 
 	kfree(log);
 	return err;
-- 
2.54.0

[PATCH bpf-next 3/5] libbpf: Add OPTS_VALID() for log_opts in bpf_map_create

[Leon Hwang]

There should be an OPTS_VALID() check for log_opts before extracting its
fields.

If no such OPTS_VALID() check and an application compiled against a future
libbpf header passes a log_opts with new, non-zero fields to libbpf.so,
those fields will be ignored silently.

Fixes: 702259006f93 ("libbpf: Add syscall common attributes support for map_create")
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 tools/lib/bpf/bpf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 483c02cf21d1..3cd705802330 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -246,6 +246,9 @@ int bpf_map_create(enum bpf_map_type map_type,
 	attr.excl_prog_hash_size = OPTS_GET(opts, excl_prog_hash_size, 0);
 
 	log_opts = OPTS_GET(opts, log_opts, NULL);
+	if (!OPTS_VALID(log_opts, bpf_log_opts))
+		return libbpf_err(-EINVAL);
+
 	if (log_opts && feat_supported(NULL, FEAT_BPF_SYSCALL_COMMON_ATTRS)) {
 		memset(&attr_common, 0, attr_common_sz);
 		attr_common.log_buf = ptr_to_u64(OPTS_GET(log_opts, buf, NULL));
-- 
2.54.0

[PATCH bpf-next 4/5] selftests/bpf: Use -1 as token_fd in map create failure test

[Leon Hwang]

Because 0xFF can be an open BPF token fd in the test runner that will fail
test_invalid_token_fd(), change token_fd from 0xFF to -1 to avoid such
test failure.

Fixes: f675483cac1d ("selftests/bpf: Add tests to verify map create failure log")
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 tools/testing/selftests/bpf/prog_tests/map_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
index 5c61c8e37306..b0b902d5783d 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_init.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_init.c
@@ -306,7 +306,7 @@ static void test_invalid_token_fd(void)
 	const char *msg = "Invalid map_token_fd.\n";
 	LIBBPF_OPTS(bpf_map_create_opts, opts,
 		    .map_flags = BPF_F_TOKEN_FD,
-		    .token_fd = 0xFF,
+		    .token_fd = -1,
 	);
 
 	test_map_create_array(&opts, msg);
-- 
2.54.0

[PATCH bpf-next 5/5] selftests/bpf: Add test to verify checking padding bytes for BPF syscall common attributes

[Leon Hwang]

Add a test to verify that the tailing padding 4 bytes are checked in
syscall.c::__sys_bpf() using bpf_check_uarg_tail_zero().

Without the fix, the test fails with:

 test_common_attr_padding:FAIL:syscall unexpected syscall: actual 4 >= expected 0
 #213/12  map_create_failure/common_attr_padding:FAIL

Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 .../selftests/bpf/prog_tests/map_init.c       | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
index b0b902d5783d..c804c3ce9be9 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_init.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_init.c
@@ -353,6 +353,30 @@ static void test_excl_prog_hash_size_2(void)
 	test_map_create_array(&opts, msg);
 }
 
+static void test_common_attr_padding(void)
+{
+	struct bpf_common_attr_fake {
+		__u8 attrs[offsetofend(struct bpf_common_attr, log_true_size)];
+		__u32 pad;
+	} attr_common = {
+		.pad = 1,
+	};
+	union bpf_attr attr = {
+		.map_type    = BPF_MAP_TYPE_ARRAY,
+		.key_size    = 4,
+		.value_size  = 4,
+		.max_entries = 1,
+	};
+	int fd;
+
+	fd = syscall(__NR_bpf, BPF_MAP_CREATE | BPF_COMMON_ATTRS, &attr, sizeof(attr), &attr_common,
+		     sizeof(attr_common));
+	if (!ASSERT_LT(fd, 0, "syscall"))
+		close(fd);
+	else
+		ASSERT_EQ(errno, E2BIG, "errno");
+}
+
 void test_map_create_failure(void)
 {
 	if (test__start_subtest("invalid_vmlinux_value_type_id_struct_ops"))
@@ -377,4 +401,6 @@ void test_map_create_failure(void)
 		test_excl_prog_hash_size_1();
 	if (test__start_subtest("invalid_excl_prog_hash_size_2"))
 		test_excl_prog_hash_size_2();
+	if (test__start_subtest("common_attr_padding"))
+		test_common_attr_padding();
 }
-- 
2.54.0

Re: [PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1972 bytes --]

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 83de8fb9b9aa..322865a88b3a 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1647,11 +1647,18 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_at
>
>  	/* preserve original error even if log finalization is successful */
>  	ret = bpf_log_attr_finalize(&attr_log, log);
> -	if (ret) {
> -		if (err >= 0)
> -			close_fd(err);
> +	if (ret && err < 0)
> +		/*
> +		 * Failed to finalize the log.
> +		 * Should not close_fd(err) here. Since the bpf_map_new_fd()
> +		 * has published the map fd, if a concurrent thread closes the
> +		 * fd, then opens new, unrelated file that receives the exact
> +		 * same fd number, close_fd(err) might inadvertently close the
> +		 * unrelated file.
> +		 * As a trade-off, override the err only when failed to finalize
> +		 * the log and failed to create map.
> +		 */
>  		err = ret;

In the v13 review thread, Alexei Starovoitov suggested moving
bpf_log_attr_finalize() earlier in the flow to eliminate the race
window entirely:

  "bot is correct. Let's avoid these races. Pls move
   bpf_log_attr_finalize() into map_create and do it before
   security_bpf_map_create."

  https://lore.kernel.org/bpf/CAADnVQ+XR3kyqizGgGhtG5xiBu3oK3O+COUMfPDQDBSJbm=56A@mail.gmail.com/

Does the current approach fully address the race concern? While removing
close_fd() prevents the specific file descriptor hijacking issue, the
suggestion was to finalize the log before security_bpf_map_create(),
which would avoid creating the race window in the first place.

Is there a reason why the log finalization cannot be moved earlier as
Alexei suggested?

>
>  	kfree(log);
>  	return err;


---
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/26042398224

Re: [PATCH bpf-next 1/5] bpf: Check tail zero of bpf_common_attr using offsetofend

[Mykyta Yatsenko]

On 5/18/26 3:54 PM, Leon Hwang wrote:
> Because of the 8-byte alignment, the compiler will pad struct
> bpf_common_attr to 24 bytes. That said, sizeof(attr_common) is 24 instead
> of 20.
> 
> When check tail zero using sizeof(attr_common) in
> bpf_check_uarg_tail_zero(), there will be 4 bytes that won't be checked.
> 
> To also check the padding 4 bytes, replace sizeof(attr_common) with
> offsetofend(struct bpf_common_attr, log_true_size).
> 
> Fixes: f28771c0691b ("bpf: Extend BPF syscall with common attributes support")
> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> ---
>  kernel/bpf/syscall.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6600e126fbfb..83de8fb9b9aa 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -6278,7 +6278,9 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
>  
>  	memset(&attr_common, 0, sizeof(attr_common));
>  	if (cmd & BPF_COMMON_ATTRS) {
> -		err = bpf_check_uarg_tail_zero(uattr_common, sizeof(attr_common), size_common);
> +		err = bpf_check_uarg_tail_zero(uattr_common,
> +					       offsetofend(struct bpf_common_attr, log_true_size),
> +					       size_common);

The change looks correct.

It looks like similar behavior exists in 2 other places of this file:
bpf_map_get_info_by_fd()
bpf_prog_get_info_by_fd()

Does it make sense to fix those too, just to make sure agents follow the
pattern correctly next time?

>  		if (err)
>  			return err;
>  

Re: [PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create()

[Mykyta Yatsenko]

On 5/18/26 3:54 PM, Leon Hwang wrote:
> Because there is time gap between bpf_map_new_fd() and close_fd(), a
> concurrent thread is able to close the new fd and opens a new, unrelated
> file with the exact same fd number. Thereafter, this close_fd() might
> inadvertently close the unrelated file.
> 
> To avoid such regression, drop close_fd() and override err when failed to
> create map and failed to finalize the log.
> 
> In other word, when succeed in creating map but fail to finalize log,
> users will get the map fd instead of the finalization error.
> 
> Fixes: 49f9b2b2a18c ("bpf: Add syscall common attributes support for map_create")
> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> ---
>  kernel/bpf/syscall.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 83de8fb9b9aa..322865a88b3a 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1647,11 +1647,18 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_at
>  
>  	/* preserve original error even if log finalization is successful */
>  	ret = bpf_log_attr_finalize(&attr_log, log);
> -	if (ret) {
> -		if (err >= 0)
> -			close_fd(err);
> +	if (ret && err < 0)
> +		/*
> +		 * Failed to finalize the log.
> +		 * Should not close_fd(err) here. Since the bpf_map_new_fd()

nit: you can't close_fd(err) here, because err < 0? The comment overall appears to
explain why we are making this change right now, rather than why this works this way.

If map_crate() failed with error code and then log finalization failed, do we really
want to override error code? It sounds like map_create error is more important.

> +		 * has published the map fd, if a concurrent thread closes the
> +		 * fd, then opens new, unrelated file that receives the exact
> +		 * same fd number, close_fd(err) might inadvertently close the
> +		 * unrelated file.
> +		 * As a trade-off, override the err only when failed to finalize
> +		 * the log and failed to create map.
> +		 */
>  		err = ret;
> -	}
>  
>  	kfree(log);
>  	return err;

Re: [PATCH bpf-next 0/5] bpf: Follow-up fixes for BPF syscall common attributes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Mon, 18 May 2026 22:54:41 +0800 you wrote:
> Address sashiko reviews for BPF syscall common attributes series.
> 
> 1. The tailing padding bytes in struct bpf_common_attr should be
>    checked. [1]
> 2. There was a concurrent regression in syscall.c::map_create(). [2]
> 3. OPTS_VALID() was missing to validate the nested struct bpf_log_opts
>    in libbpf. [3]
> 4. The token_fd should be -1 to avoid a valid token fd in test. [4]
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/5] bpf: Check tail zero of bpf_common_attr using offsetofend
    https://git.kernel.org/bpf/bpf-next/c/f05ddc6771c4
  - [bpf-next,2/5] bpf: Fix concurrent regression in map_create()
    (no matching commit)
  - [bpf-next,3/5] libbpf: Add OPTS_VALID() for log_opts in bpf_map_create
    https://git.kernel.org/bpf/bpf-next/c/b4844cb6d1ec
  - [bpf-next,4/5] selftests/bpf: Use -1 as token_fd in map create failure test
    https://git.kernel.org/bpf/bpf-next/c/652f0c2c999d
  - [bpf-next,5/5] selftests/bpf: Add test to verify checking padding bytes for BPF syscall common attributes
    https://git.kernel.org/bpf/bpf-next/c/7732ad2412fd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf-next 1/5] bpf: Check tail zero of bpf_common_attr using offsetofend

[Leon Hwang]

On 19/5/26 00:14, Mykyta Yatsenko wrote:
> 
> 
> On 5/18/26 3:54 PM, Leon Hwang wrote:
[...]
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 6600e126fbfb..83de8fb9b9aa 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -6278,7 +6278,9 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
>>  
>>  	memset(&attr_common, 0, sizeof(attr_common));
>>  	if (cmd & BPF_COMMON_ATTRS) {
>> -		err = bpf_check_uarg_tail_zero(uattr_common, sizeof(attr_common), size_common);
>> +		err = bpf_check_uarg_tail_zero(uattr_common,
>> +					       offsetofend(struct bpf_common_attr, log_true_size),
>> +					       size_common);
> 
> The change looks correct.
> 
> It looks like similar behavior exists in 2 other places of this file:
> bpf_map_get_info_by_fd()
> bpf_prog_get_info_by_fd()
> 

Verify by 'pahole -C bpf_map_info/bpf_prog_info'.

Yes, there are 4 bytes padding at the end of these two structs.

> Does it make sense to fix those too, just to make sure agents follow the
> pattern correctly next time?
> 

Will post separate patches to fix them.

Thanks,
Leon

Re: [PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create()

[Leon Hwang]

On 19/5/26 00:43, Mykyta Yatsenko wrote:
> 
> 
> On 5/18/26 3:54 PM, Leon Hwang wrote:
[...]
>>  	/* preserve original error even if log finalization is successful */
>>  	ret = bpf_log_attr_finalize(&attr_log, log);
>> -	if (ret) {
>> -		if (err >= 0)
>> -			close_fd(err);
>> +	if (ret && err < 0)
>> +		/*
>> +		 * Failed to finalize the log.
>> +		 * Should not close_fd(err) here. Since the bpf_map_new_fd()
> 
> nit: you can't close_fd(err) here, because err < 0? The comment overall appears to
> explain why we are making this change right now, rather than why this works this way.
> 

Will update the comment to

		/*
		 * Failed to finalize the log.
		 *
		 * Should not close_fd(err) here by
		 *
		 *  if (ret) {
		 *      if (err >= 0)
		 *              close_fd(err);
		 *      err = ret;
		 *  }
		 *
		 * Since the bpf_map_new_fd() has published the map fd,
		 * if a concurrent thread closes the fd, then opens new,
		 * unrelated file that receives the exact same fd
		 * number, close_fd(err) might inadvertently close the
		 * unrelated file.
		 *
		 * As a trade-off, override the err only when failed to
		 * finalize the log and failed to create map.
		 */

The comment is to explain the reason for overriding err.

> If map_crate() failed with error code and then log finalization failed, do we really
> want to override error code? It sounds like map_create error is more important.
> 

In that case, there are two error codes that should be returned to user
space. How to achieve it?

Since we should not ignore any error code of them, can we report the
error code of __map_create() failure by adding an error attr to struct
bpf_common_attr? Hmm, seems not a good idea.

Any idea?

Thanks,
Leon

>> +		 * has published the map fd, if a concurrent thread closes the
>> +		 * fd, then opens new, unrelated file that receives the exact
>> +		 * same fd number, close_fd(err) might inadvertently close the
>> +		 * unrelated file.
>> +		 * As a trade-off, override the err only when failed to finalize
>> +		 * the log and failed to create map.
>> +		 */
>>  		err = ret;
>> -	}
>>  
>>  	kfree(log);
>>  	return err;
> 

Re: [PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create()

[Leon Hwang]

On 18/5/26 23:40, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 83de8fb9b9aa..322865a88b3a 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -1647,11 +1647,18 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_at
>>
>>  	/* preserve original error even if log finalization is successful */
>>  	ret = bpf_log_attr_finalize(&attr_log, log);
>> -	if (ret) {
>> -		if (err >= 0)
>> -			close_fd(err);
>> +	if (ret && err < 0)
>> +		/*
>> +		 * Failed to finalize the log.
>> +		 * Should not close_fd(err) here. Since the bpf_map_new_fd()
>> +		 * has published the map fd, if a concurrent thread closes the
>> +		 * fd, then opens new, unrelated file that receives the exact
>> +		 * same fd number, close_fd(err) might inadvertently close the
>> +		 * unrelated file.
>> +		 * As a trade-off, override the err only when failed to finalize
>> +		 * the log and failed to create map.
>> +		 */
>>  		err = ret;
> 
> In the v13 review thread, Alexei Starovoitov suggested moving
> bpf_log_attr_finalize() earlier in the flow to eliminate the race
> window entirely:
> 
>   "bot is correct. Let's avoid these races. Pls move
>    bpf_log_attr_finalize() into map_create and do it before
>    security_bpf_map_create."
> 
>   https://lore.kernel.org/bpf/CAADnVQ+XR3kyqizGgGhtG5xiBu3oK3O+COUMfPDQDBSJbm=56A@mail.gmail.com/
> 
> Does the current approach fully address the race concern? While removing
> close_fd() prevents the specific file descriptor hijacking issue, the
> suggestion was to finalize the log before security_bpf_map_create(),
> which would avoid creating the race window in the first place.
> 
> Is there a reason why the log finalization cannot be moved earlier as
> Alexei suggested?
> 
I've tried his suggestion, attached below.

However, I think there should be a simpler approach to fix it.

I preferred this simple one, so I posted it.

Should I follow the suggestion instead of this trade-off approach?

Thanks,
Leon

---

From 7ea7ab53d24b7cf34bd2775ef5c871aa312d07c6 Mon Sep 17 00:00:00 2001
From: Leon Hwang <leon.hwang@linux.dev>
Date: Tue, 19 May 2026 10:32:01 +0800
Subject: [PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create()

Because there is time gap between bpf_map_new_fd() and close_fd(), a
concurrent thread is able to close the new fd and opens a new, unrelated
file with the exact same fd number. Thereafter, this close_fd() might
inadvertently close the unrelated file.

To avoid such regression, add bpf_log_attr_finalize() before
security_bpf_map_create() to report log_true_size on check-success path.

While keep the original bpf_log_attr_finalize() and avoid finalizing
bpf_log_attr twice, guard the finalization using attr_log->finalized.

Fixes: 49f9b2b2a18c ("bpf: Add syscall common attributes support for
map_create")
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 include/linux/bpf_verifier.h |  1 +
 kernel/bpf/log.c             |  4 ++++
 kernel/bpf/syscall.c         | 21 ++++++++++++++-------
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 20c421b43849..a10ef58bb6ea 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -788,6 +788,7 @@ struct bpf_log_attr {
 	u32 level;
 	u32 offsetof_true_size;
 	bpfptr_t uattr;
+	bool finalized;
 };

 int bpf_log_attr_init(struct bpf_log_attr *log, u64 log_buf, u32
log_size, u32 log_level,
diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
index 62fe6ed18374..71e3035515cd 100644
--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c
@@ -894,6 +894,10 @@ int bpf_log_attr_finalize(struct bpf_log_attr
*attr, struct bpf_verifier_log *lo
 	u32 log_true_size;
 	int err;

+	if (attr->finalized)
+		return 0;
+	attr->finalized = true;
+
 	err = bpf_vlog_finalize(log, &log_true_size);

 	if (attr->offsetof_true_size &&
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 83de8fb9b9aa..c117e2286cf0 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1359,7 +1359,8 @@ static int map_check_btf(struct bpf_map *map,
struct bpf_token *token,

 #define BPF_MAP_CREATE_LAST_FIELD excl_prog_hash_size
 /* called via syscall */
-static int __map_create(union bpf_attr *attr, bpfptr_t uattr, struct
bpf_verifier_log *log)
+static int __map_create(union bpf_attr *attr, bpfptr_t uattr, struct
bpf_verifier_log *log,
+			struct bpf_log_attr *attr_log)
 {
 	const struct bpf_map_ops *ops;
 	struct bpf_token *token = NULL;
@@ -1598,6 +1599,10 @@ static int __map_create(union bpf_attr *attr,
bpfptr_t uattr, struct bpf_verifie
 		goto free_map;
 	}

+	err = bpf_log_attr_finalize(attr_log, log);
+	if (err)
+		goto free_map;
+
 	err = security_bpf_map_create(map, attr, token, uattr.is_kernel);
 	if (err)
 		goto free_map_sec;
@@ -1643,15 +1648,17 @@ static int map_create(union bpf_attr *attr,
bpfptr_t uattr, struct bpf_common_at
 	if (IS_ERR(log))
 		return PTR_ERR(log);

-	err = __map_create(attr, uattr, log);
+	err = __map_create(attr, uattr, log, &attr_log);

-	/* preserve original error even if log finalization is successful */
+	/* preserve original error even if log finalization is
+	successful */
+	/*
+	 * No duplicated finalization here because of attr_log->finalized
+	 * guard in bpf_log_attr_finalize().
+	 */
 	ret = bpf_log_attr_finalize(&attr_log, log);
-	if (ret) {
-		if (err >= 0)
-			close_fd(err);
+	if (ret)
 		err = ret;
-	}

 	kfree(log);
 	return err;
-- 
2.54.0

Re: [PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create()

[Alexei Starovoitov]

On Mon, May 18, 2026 at 7:48 PM Leon Hwang <leon.hwang@linux.dev> wrote:
>
> On 18/5/26 23:40, bot+bpf-ci@kernel.org wrote:
> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> >> index 83de8fb9b9aa..322865a88b3a 100644
> >> --- a/kernel/bpf/syscall.c
> >> +++ b/kernel/bpf/syscall.c
> >> @@ -1647,11 +1647,18 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_at
> >>
> >>      /* preserve original error even if log finalization is successful */
> >>      ret = bpf_log_attr_finalize(&attr_log, log);
> >> -    if (ret) {
> >> -            if (err >= 0)
> >> -                    close_fd(err);
> >> +    if (ret && err < 0)
> >> +            /*
> >> +             * Failed to finalize the log.
> >> +             * Should not close_fd(err) here. Since the bpf_map_new_fd()
> >> +             * has published the map fd, if a concurrent thread closes the
> >> +             * fd, then opens new, unrelated file that receives the exact
> >> +             * same fd number, close_fd(err) might inadvertently close the
> >> +             * unrelated file.
> >> +             * As a trade-off, override the err only when failed to finalize
> >> +             * the log and failed to create map.
> >> +             */
> >>              err = ret;
> >
> > In the v13 review thread, Alexei Starovoitov suggested moving
> > bpf_log_attr_finalize() earlier in the flow to eliminate the race
> > window entirely:
> >
> >   "bot is correct. Let's avoid these races. Pls move
> >    bpf_log_attr_finalize() into map_create and do it before
> >    security_bpf_map_create."
> >
> >   https://lore.kernel.org/bpf/CAADnVQ+XR3kyqizGgGhtG5xiBu3oK3O+COUMfPDQDBSJbm=56A@mail.gmail.com/
> >
> > Does the current approach fully address the race concern? While removing
> > close_fd() prevents the specific file descriptor hijacking issue, the
> > suggestion was to finalize the log before security_bpf_map_create(),
> > which would avoid creating the race window in the first place.
> >
> > Is there a reason why the log finalization cannot be moved earlier as
> > Alexei suggested?
> >
> I've tried his suggestion, attached below.
>
> However, I think there should be a simpler approach to fix it.
>
> I preferred this simple one, so I posted it.
>
> Should I follow the suggestion instead of this trade-off approach?
>
> Thanks,
> Leon
>
> ---
>
> From 7ea7ab53d24b7cf34bd2775ef5c871aa312d07c6 Mon Sep 17 00:00:00 2001
> From: Leon Hwang <leon.hwang@linux.dev>
> Date: Tue, 19 May 2026 10:32:01 +0800
> Subject: [PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create()
>
> Because there is time gap between bpf_map_new_fd() and close_fd(), a
> concurrent thread is able to close the new fd and opens a new, unrelated
> file with the exact same fd number. Thereafter, this close_fd() might
> inadvertently close the unrelated file.
>
> To avoid such regression, add bpf_log_attr_finalize() before
> security_bpf_map_create() to report log_true_size on check-success path.
>
> While keep the original bpf_log_attr_finalize() and avoid finalizing
> bpf_log_attr twice, guard the finalization using attr_log->finalized.
>
> Fixes: 49f9b2b2a18c ("bpf: Add syscall common attributes support for
> map_create")
> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> ---
>  include/linux/bpf_verifier.h |  1 +
>  kernel/bpf/log.c             |  4 ++++
>  kernel/bpf/syscall.c         | 21 ++++++++++++++-------
>  3 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 20c421b43849..a10ef58bb6ea 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -788,6 +788,7 @@ struct bpf_log_attr {
>         u32 level;
>         u32 offsetof_true_size;
>         bpfptr_t uattr;
> +       bool finalized;
>  };

No. That looks worse.
The suggestion was to do the finalize log _once_ before
allocating FD.
Why are you doing it twice?

Re: [PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create()

[Leon Hwang]

On 19/5/26 11:05, Alexei Starovoitov wrote:
> On Mon, May 18, 2026 at 7:48 PM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> On 18/5/26 23:40, bot+bpf-ci@kernel.org wrote:
[...]
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index 20c421b43849..a10ef58bb6ea 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -788,6 +788,7 @@ struct bpf_log_attr {
>>         u32 level;
>>         u32 offsetof_true_size;
>>         bpfptr_t uattr;
>> +       bool finalized;
>>  };
> 
> No. That looks worse.
> The suggestion was to do the finalize log _once_ before
> allocating FD.

Understand the suggestion.

When tried to finalize log once without "finalized" guard, it seemed
complicated to modify __map_create() to make sure finalize log once on
all code logic paths.

> Why are you doing it twice?

So, to simplify the change, finalize log the second time with the
"finalized" guard to cover all the check-failure paths.

After thinking hardly, the below patch is the way to make sure finalize
log once before security_bpf_map_create().

It works, but it is not good enough.

Thanks,
Leon

---

From 80f697c30e830786a57787f370299d4e3335fbbc Mon Sep 17 00:00:00 2001
From: Leon Hwang <leon.hwang@linux.dev>
Date: Tue, 19 May 2026 15:32:45 +0800
Subject: [PATCH bpf-next v2] bpf: Fix concurrent regression in map_create()

Because there is time gap between bpf_map_new_fd() and close_fd(), a
concurrent thread is able to close the new fd and opens a new, unrelated
file with the exact same fd number. Thereafter, this close_fd() might
inadvertently close the unrelated file.

To avoid such regression, do finalize log before security_bpf_map_create().

However, to achieve it, move bpf_get_file_flag(),
security_bpf_map_create(), bpf_map_alloc_id(), and bpf_map_new_fd() from
__map_create() to map_create(). And, rename __map_create() to
map_create_alloc() meanwhile.

Fixes: 49f9b2b2a18c ("bpf: Add syscall common attributes support for
map_create")
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 kernel/bpf/syscall.c | 79 ++++++++++++++++++++++++++------------------
 1 file changed, 46 insertions(+), 33 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 83de8fb9b9aa..e815457f5b24 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1359,7 +1359,8 @@ static int map_check_btf(struct bpf_map *map,
struct bpf_token *token,

 #define BPF_MAP_CREATE_LAST_FIELD excl_prog_hash_size
 /* called via syscall */
-static int __map_create(union bpf_attr *attr, bpfptr_t uattr, struct
bpf_verifier_log *log)
+static int map_create_alloc(union bpf_attr *attr, bpfptr_t uattr,
struct bpf_verifier_log *log,
+			    struct bpf_map **mapp, struct bpf_token **tokenp)
 {
 	const struct bpf_map_ops *ops;
 	struct bpf_token *token = NULL;
@@ -1367,7 +1368,6 @@ static int __map_create(union bpf_attr *attr,
bpfptr_t uattr, struct bpf_verifie
 	u32 map_type = attr->map_type;
 	struct bpf_map *map;
 	bool token_flag;
-	int f_flags;
 	int err;

 	err = CHECK_ATTR(BPF_MAP_CREATE);
@@ -1403,10 +1403,6 @@ static int __map_create(union bpf_attr *attr,
bpfptr_t uattr, struct bpf_verifie
 		return -EINVAL;
 	}

-	f_flags = bpf_get_file_flag(attr->map_flags);
-	if (f_flags < 0)
-		return f_flags;
-
 	if (numa_node != NUMA_NO_NODE &&
 	    ((unsigned int)numa_node >= nr_node_ids ||
 	     !node_online(numa_node))) {
@@ -1598,6 +1594,48 @@ static int __map_create(union bpf_attr *attr,
bpfptr_t uattr, struct bpf_verifie
 		goto free_map;
 	}

+	*mapp = map;
+	*tokenp = token;
+	return 0;
+
+free_map:
+	bpf_map_free(map);
+put_token:
+	bpf_token_put(token);
+	return err;
+}
+
+static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct
bpf_common_attr *attr_common,
+		      bpfptr_t uattr_common, u32 size_common)
+{
+	struct bpf_token *token = NULL;
+	struct bpf_verifier_log *log;
+	struct bpf_log_attr attr_log;
+	struct bpf_map *map = NULL;
+	int err, ret;
+	int f_flags;
+
+	log = bpf_log_attr_create_vlog(&attr_log, attr_common, uattr_common,
size_common);
+	if (IS_ERR(log))
+		return PTR_ERR(log);
+
+	err = map_create_alloc(attr, uattr, log, &map, &token);
+
+	/* preserve original error even if log finalization is successful */
+	ret = bpf_log_attr_finalize(&attr_log, log);
+	if (ret)
+		err = ret;
+	kfree(log);
+
+	if (err)
+		goto free_map;
+
+	f_flags = bpf_get_file_flag(attr->map_flags);
+	if (f_flags < 0) {
+		err = f_flags;
+		goto free_map;
+	}
+
 	err = security_bpf_map_create(map, attr, token, uattr.is_kernel);
 	if (err)
 		goto free_map_sec;
@@ -1626,37 +1664,12 @@ static int __map_create(union bpf_attr *attr,
bpfptr_t uattr, struct bpf_verifie
 free_map_sec:
 	security_bpf_map_free(map);
 free_map:
-	bpf_map_free(map);
-put_token:
+	if (map)
+		bpf_map_free(map);
 	bpf_token_put(token);
 	return err;
 }

-static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct
bpf_common_attr *attr_common,
-		      bpfptr_t uattr_common, u32 size_common)
-{
-	struct bpf_verifier_log *log;
-	struct bpf_log_attr attr_log;
-	int err, ret;
-
-	log = bpf_log_attr_create_vlog(&attr_log, attr_common, uattr_common,
size_common);
-	if (IS_ERR(log))
-		return PTR_ERR(log);
-
-	err = __map_create(attr, uattr, log);
-
-	/* preserve original error even if log finalization is successful */
-	ret = bpf_log_attr_finalize(&attr_log, log);
-	if (ret) {
-		if (err >= 0)
-			close_fd(err);
-		err = ret;
-	}
-
-	kfree(log);
-	return err;
-}
-
 void bpf_map_inc(struct bpf_map *map)
 {
 	atomic64_inc(&map->refcnt);
-- 
2.54.0