diff mbox series

[bpf-next,v3,2/2] bpf: Check return from set_memory_rox()

Message ID 4d7cc25e937403ac61ae61be06f998f27e631a65.1710522112.git.christophe.leroy@csgroup.eu (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series [bpf-next,v3,1/2] bpf: Remove arch_unprotect_bpf_trampoline() | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 fail Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-14 fail Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 7482 this patch: 7482
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 28 of 28 maintainers
netdev/build_clang success Errors and warnings before: 2244 this patch: 2244
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 7870 this patch: 7870
netdev/checkpatch warning WARNING: line length of 85 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 6 this patch: 6
netdev/source_inline success Was 0 now: 0

Commit Message

Christophe Leroy March 15, 2024, 5:06 p.m. UTC 
arch_protect_bpf_trampoline() and alloc_new_pack() call
set_memory_rox() which can fail, leading to unprotected memory.

Take into account return from set_memory_rox() function and add
__must_check flag to arch_protect_bpf_trampoline().

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
v3:
- Rebased and handled conflict in kernel/bpf/bpf_struct_ops.c

v2:
- Move list_add_tail(&pack->list, &pack_list) at the end of alloc_new_pack()
- Split 2 lines that are reported longer than 80 chars by BPF patchwork's checkpatch report.
---
 arch/arm64/net/bpf_jit_comp.c  |  3 ++-
 arch/x86/net/bpf_jit_comp.c    |  3 ++-
 include/linux/bpf.h            |  2 +-
 kernel/bpf/bpf_struct_ops.c    |  7 +++++--
 kernel/bpf/core.c              | 29 ++++++++++++++++++++++-------
 kernel/bpf/trampoline.c        |  8 +++++---
 net/bpf/bpf_dummy_struct_ops.c |  4 +++-
 7 files changed, 40 insertions(+), 16 deletions(-)

Comments

Kuifeng Lee March 15, 2024, 6:11 p.m. UTC | #1 
On 3/15/24 10:06, Christophe Leroy wrote:
> arch_protect_bpf_trampoline() and alloc_new_pack() call
> set_memory_rox() which can fail, leading to unprotected memory.
> 
> Take into account return from set_memory_rox() function and add
> __must_check flag to arch_protect_bpf_trampoline().
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> ---
> v3:
> - Rebased and handled conflict in kernel/bpf/bpf_struct_ops.c
> 
> v2:
> - Move list_add_tail(&pack->list, &pack_list) at the end of alloc_new_pack()
> - Split 2 lines that are reported longer than 80 chars by BPF patchwork's checkpatch report.
> ---
>   arch/arm64/net/bpf_jit_comp.c  |  3 ++-
>   arch/x86/net/bpf_jit_comp.c    |  3 ++-
>   include/linux/bpf.h            |  2 +-
>   kernel/bpf/bpf_struct_ops.c    |  7 +++++--
>   kernel/bpf/core.c              | 29 ++++++++++++++++++++++-------
>   kernel/bpf/trampoline.c        |  8 +++++---
>   net/bpf/bpf_dummy_struct_ops.c |  4 +++-
>   7 files changed, 40 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 132c8ffba109..bc16eb694657 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -2176,8 +2176,9 @@ void arch_free_bpf_trampoline(void *image, unsigned int size)
>   	bpf_prog_pack_free(image, size);
>   }
>   
> -void arch_protect_bpf_trampoline(void *image, unsigned int size)
> +int arch_protect_bpf_trampoline(void *image, unsigned int size)
>   {
> +	return 0;
>   }
>   
>   int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *ro_image,
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 7a56d2d84512..4900b1ee019f 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -3004,8 +3004,9 @@ void arch_free_bpf_trampoline(void *image, unsigned int size)
>   	bpf_prog_pack_free(image, size);
>   }
>   
> -void arch_protect_bpf_trampoline(void *image, unsigned int size)
> +int arch_protect_bpf_trampoline(void *image, unsigned int size)
>   {
> +	return 0;
>   }
>   
>   int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index d89bdefb42e2..17843e66a1d3 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1116,7 +1116,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
>   				void *func_addr);
>   void *arch_alloc_bpf_trampoline(unsigned int size);
>   void arch_free_bpf_trampoline(void *image, unsigned int size);
> -void arch_protect_bpf_trampoline(void *image, unsigned int size);
> +int __must_check arch_protect_bpf_trampoline(void *image, unsigned int size);
>   int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
>   			     struct bpf_tramp_links *tlinks, void *func_addr);
>   
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 43356faaa057..ca1d9b87c475 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -742,8 +742,11 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
>   		if (err)
>   			goto reset_unlock;
>   	}
> -	for (i = 0; i < st_map->image_pages_cnt; i++)
> -		arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
> +	for (i = 0; i < st_map->image_pages_cnt && !err; i++)
> +		err = arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
> +
> +	if (err)

nit: Can it be more specific? I mean to check err < 0, so we can reason
that this function never returns a positive value other than 0.

> +		goto reset_unlock;
>   
>   	if (st_map->map.map_flags & BPF_F_LINK) {
>   		err = 0;
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 63f100def31b..1e761c3c66db 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -908,23 +908,31 @@ static LIST_HEAD(pack_list);
>   static struct bpf_prog_pack *alloc_new_pack(bpf_jit_fill_hole_t bpf_fill_ill_insns)
>   {
>   	struct bpf_prog_pack *pack;
> +	int err;
>   
>   	pack = kzalloc(struct_size(pack, bitmap, BITS_TO_LONGS(BPF_PROG_CHUNK_COUNT)),
>   		       GFP_KERNEL);
>   	if (!pack)
>   		return NULL;
>   	pack->ptr = bpf_jit_alloc_exec(BPF_PROG_PACK_SIZE);
> -	if (!pack->ptr) {
> -		kfree(pack);
> -		return NULL;
> -	}
> +	if (!pack->ptr)
> +		goto out;
>   	bpf_fill_ill_insns(pack->ptr, BPF_PROG_PACK_SIZE);
>   	bitmap_zero(pack->bitmap, BPF_PROG_PACK_SIZE / BPF_PROG_CHUNK_SIZE);
> -	list_add_tail(&pack->list, &pack_list);
>   
>   	set_vm_flush_reset_perms(pack->ptr);
> -	set_memory_rox((unsigned long)pack->ptr, BPF_PROG_PACK_SIZE / PAGE_SIZE);
> +	err = set_memory_rox((unsigned long)pack->ptr,
> +			     BPF_PROG_PACK_SIZE / PAGE_SIZE);
> +	if (err)
> +		goto out_free;
> +	list_add_tail(&pack->list, &pack_list);
>   	return pack;
> +
> +out_free:
> +	bpf_jit_free_exec(pack->ptr);
> +out:
> +	kfree(pack);
> +	return NULL;
>   }
>   
>   void *bpf_prog_pack_alloc(u32 size, bpf_jit_fill_hole_t bpf_fill_ill_insns)
> @@ -939,9 +947,16 @@ void *bpf_prog_pack_alloc(u32 size, bpf_jit_fill_hole_t bpf_fill_ill_insns)
>   		size = round_up(size, PAGE_SIZE);
>   		ptr = bpf_jit_alloc_exec(size);
>   		if (ptr) {
> +			int err;
> +
>   			bpf_fill_ill_insns(ptr, size);
>   			set_vm_flush_reset_perms(ptr);
> -			set_memory_rox((unsigned long)ptr, size / PAGE_SIZE);
> +			err = set_memory_rox((unsigned long)ptr,
> +					     size / PAGE_SIZE);
> +			if (err) {
> +				bpf_jit_free_exec(ptr);
> +				ptr = NULL;
> +			}
>   		}
>   		goto out;
>   	}
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 04fd1abd3661..cc50607f8d8c 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -456,7 +456,9 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>   	if (err < 0)
>   		goto out_free;

Same here

>   
> -	arch_protect_bpf_trampoline(im->image, im->size);
> +	err = arch_protect_bpf_trampoline(im->image, im->size);
> +	if (err)
> +		goto out_free;
>   
>   	WARN_ON(tr->cur_image && total == 0);
>   	if (tr->cur_image)
> @@ -1072,10 +1074,10 @@ void __weak arch_free_bpf_trampoline(void *image, unsigned int size)
>   	bpf_jit_free_exec(image);
>   }
>   
> -void __weak arch_protect_bpf_trampoline(void *image, unsigned int size)
> +int __weak arch_protect_bpf_trampoline(void *image, unsigned int size)
>   {
>   	WARN_ON_ONCE(size > PAGE_SIZE);
> -	set_memory_rox((long)image, 1);
> +	return set_memory_rox((long)image, 1);
>   }
>   
>   int __weak arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c
> index de33dc1b0daa..25b75844891a 100644
> --- a/net/bpf/bpf_dummy_struct_ops.c
> +++ b/net/bpf/bpf_dummy_struct_ops.c
> @@ -133,7 +133,9 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
>   	if (err < 0)
>   		goto out;
>   
> -	arch_protect_bpf_trampoline(image, PAGE_SIZE);
> +	err = arch_protect_bpf_trampoline(image, PAGE_SIZE);
> +	if (err)

Same here.

> +		goto out;
>   	prog_ret = dummy_ops_call_op(image, args);
>   
>   	err = dummy_ops_copy_args(args);
Martin KaFai Lau March 15, 2024, 6:34 p.m. UTC | #2 
On 3/15/24 11:11 AM, Kui-Feng Lee wrote:
>> --- a/kernel/bpf/bpf_struct_ops.c
>> +++ b/kernel/bpf/bpf_struct_ops.c
>> @@ -742,8 +742,11 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map 
>> *map, void *key,
>>           if (err)
>>               goto reset_unlock;
>>       }
>> -    for (i = 0; i < st_map->image_pages_cnt; i++)
>> -        arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
>> +    for (i = 0; i < st_map->image_pages_cnt && !err; i++)
>> +        err = arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
>> +
>> +    if (err)
> 
> nit: Can it be more specific? I mean to check err < 0, so we can reason
> that this function never returns a positive value other than 0.

I think "if (err)" is fine. It is pretty common in other places of the kernel.

Checking "(err < 0)" may actually mean the return value could be positive. At 
least it is how bpf_struct_ops.c is using "(err < 0)".

[ An unrelated side note is another (err < 0) check in bpf_struct_ops.c could 
have been changed after the recent changes in bpf_struct_ops_prepare_trampoline 
which no longer return +val ].
Martin KaFai Lau March 15, 2024, 8:55 p.m. UTC | #3 
On 3/15/24 10:06 AM, Christophe Leroy wrote:
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 43356faaa057..ca1d9b87c475 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -742,8 +742,11 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
>   		if (err)
>   			goto reset_unlock;
>   	}
> -	for (i = 0; i < st_map->image_pages_cnt; i++)
> -		arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
> +	for (i = 0; i < st_map->image_pages_cnt && !err; i++)
> +		err = arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
> +
> +	if (err)
> +		goto reset_unlock;

This part does not look right. The "if (err)" check should be inside the for loop.

pw-bot: cr
Martin KaFai Lau March 15, 2024, 9:11 p.m. UTC | #4 
On 3/15/24 1:55 PM, Martin KaFai Lau wrote:
> On 3/15/24 10:06 AM, Christophe Leroy wrote:
>> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
>> index 43356faaa057..ca1d9b87c475 100644
>> --- a/kernel/bpf/bpf_struct_ops.c
>> +++ b/kernel/bpf/bpf_struct_ops.c
>> @@ -742,8 +742,11 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map 
>> *map, void *key,
>>           if (err)
>>               goto reset_unlock;
>>       }
>> -    for (i = 0; i < st_map->image_pages_cnt; i++)
>> -        arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
>> +    for (i = 0; i < st_map->image_pages_cnt && !err; i++)
>> +        err = arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
>> +
>> +    if (err)
>> +        goto reset_unlock;
> 
> This part does not look right. The "if (err)" check should be inside the for loop.

ah. Please ignore. missed the "!err" in the for loop.
Martin KaFai Lau March 16, 2024, 12:56 a.m. UTC | #5 
On 3/15/24 2:11 PM, Martin KaFai Lau wrote:
> On 3/15/24 1:55 PM, Martin KaFai Lau wrote:
>> On 3/15/24 10:06 AM, Christophe Leroy wrote:
>>> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
>>> index 43356faaa057..ca1d9b87c475 100644
>>> --- a/kernel/bpf/bpf_struct_ops.c
>>> +++ b/kernel/bpf/bpf_struct_ops.c
>>> @@ -742,8 +742,11 @@ static long bpf_struct_ops_map_update_elem(struct 
>>> bpf_map *map, void *key,
>>>           if (err)
>>>               goto reset_unlock;
>>>       }
>>> -    for (i = 0; i < st_map->image_pages_cnt; i++)
>>> -        arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
>>> +    for (i = 0; i < st_map->image_pages_cnt && !err; i++)

I was about to apply but I still think checking "&& !err" is not right given how 
"err" is used in the earlier code of this function.

The err may not be 0 in the first iteration of this for loop. Take a look at the 
"if (err > 0)" check in the "for_each_member(i, t, member)" loop above.

>>> +        err = arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
>>> +
>>> +    if (err)
>>> +        goto reset_unlock;
>>
>> This part does not look right. The "if (err)" check should be inside the for 
>> loop.

Instead of adding an extra "err = 0;" before the for loop. It is better to move 
this "if (err) goto reset_unlock;" into the for loop and remove the "&& !err" 
test above.

> 
> ah. Please ignore. missed the "!err" in the for loop.
diff mbox series

Patch

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 132c8ffba109..bc16eb694657 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -2176,8 +2176,9 @@  void arch_free_bpf_trampoline(void *image, unsigned int size)
 	bpf_prog_pack_free(image, size);
 }
 
-void arch_protect_bpf_trampoline(void *image, unsigned int size)
+int arch_protect_bpf_trampoline(void *image, unsigned int size)
 {
+	return 0;
 }
 
 int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *ro_image,
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 7a56d2d84512..4900b1ee019f 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -3004,8 +3004,9 @@  void arch_free_bpf_trampoline(void *image, unsigned int size)
 	bpf_prog_pack_free(image, size);
 }
 
-void arch_protect_bpf_trampoline(void *image, unsigned int size)
+int arch_protect_bpf_trampoline(void *image, unsigned int size)
 {
+	return 0;
 }
 
 int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index d89bdefb42e2..17843e66a1d3 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1116,7 +1116,7 @@  int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
 				void *func_addr);
 void *arch_alloc_bpf_trampoline(unsigned int size);
 void arch_free_bpf_trampoline(void *image, unsigned int size);
-void arch_protect_bpf_trampoline(void *image, unsigned int size);
+int __must_check arch_protect_bpf_trampoline(void *image, unsigned int size);
 int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
 			     struct bpf_tramp_links *tlinks, void *func_addr);
 
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 43356faaa057..ca1d9b87c475 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -742,8 +742,11 @@  static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
 		if (err)
 			goto reset_unlock;
 	}
-	for (i = 0; i < st_map->image_pages_cnt; i++)
-		arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
+	for (i = 0; i < st_map->image_pages_cnt && !err; i++)
+		err = arch_protect_bpf_trampoline(st_map->image_pages[i], PAGE_SIZE);
+
+	if (err)
+		goto reset_unlock;
 
 	if (st_map->map.map_flags & BPF_F_LINK) {
 		err = 0;
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 63f100def31b..1e761c3c66db 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -908,23 +908,31 @@  static LIST_HEAD(pack_list);
 static struct bpf_prog_pack *alloc_new_pack(bpf_jit_fill_hole_t bpf_fill_ill_insns)
 {
 	struct bpf_prog_pack *pack;
+	int err;
 
 	pack = kzalloc(struct_size(pack, bitmap, BITS_TO_LONGS(BPF_PROG_CHUNK_COUNT)),
 		       GFP_KERNEL);
 	if (!pack)
 		return NULL;
 	pack->ptr = bpf_jit_alloc_exec(BPF_PROG_PACK_SIZE);
-	if (!pack->ptr) {
-		kfree(pack);
-		return NULL;
-	}
+	if (!pack->ptr)
+		goto out;
 	bpf_fill_ill_insns(pack->ptr, BPF_PROG_PACK_SIZE);
 	bitmap_zero(pack->bitmap, BPF_PROG_PACK_SIZE / BPF_PROG_CHUNK_SIZE);
-	list_add_tail(&pack->list, &pack_list);
 
 	set_vm_flush_reset_perms(pack->ptr);
-	set_memory_rox((unsigned long)pack->ptr, BPF_PROG_PACK_SIZE / PAGE_SIZE);
+	err = set_memory_rox((unsigned long)pack->ptr,
+			     BPF_PROG_PACK_SIZE / PAGE_SIZE);
+	if (err)
+		goto out_free;
+	list_add_tail(&pack->list, &pack_list);
 	return pack;
+
+out_free:
+	bpf_jit_free_exec(pack->ptr);
+out:
+	kfree(pack);
+	return NULL;
 }
 
 void *bpf_prog_pack_alloc(u32 size, bpf_jit_fill_hole_t bpf_fill_ill_insns)
@@ -939,9 +947,16 @@  void *bpf_prog_pack_alloc(u32 size, bpf_jit_fill_hole_t bpf_fill_ill_insns)
 		size = round_up(size, PAGE_SIZE);
 		ptr = bpf_jit_alloc_exec(size);
 		if (ptr) {
+			int err;
+
 			bpf_fill_ill_insns(ptr, size);
 			set_vm_flush_reset_perms(ptr);
-			set_memory_rox((unsigned long)ptr, size / PAGE_SIZE);
+			err = set_memory_rox((unsigned long)ptr,
+					     size / PAGE_SIZE);
+			if (err) {
+				bpf_jit_free_exec(ptr);
+				ptr = NULL;
+			}
 		}
 		goto out;
 	}
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 04fd1abd3661..cc50607f8d8c 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -456,7 +456,9 @@  static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
 	if (err < 0)
 		goto out_free;
 
-	arch_protect_bpf_trampoline(im->image, im->size);
+	err = arch_protect_bpf_trampoline(im->image, im->size);
+	if (err)
+		goto out_free;
 
 	WARN_ON(tr->cur_image && total == 0);
 	if (tr->cur_image)
@@ -1072,10 +1074,10 @@  void __weak arch_free_bpf_trampoline(void *image, unsigned int size)
 	bpf_jit_free_exec(image);
 }
 
-void __weak arch_protect_bpf_trampoline(void *image, unsigned int size)
+int __weak arch_protect_bpf_trampoline(void *image, unsigned int size)
 {
 	WARN_ON_ONCE(size > PAGE_SIZE);
-	set_memory_rox((long)image, 1);
+	return set_memory_rox((long)image, 1);
 }
 
 int __weak arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c
index de33dc1b0daa..25b75844891a 100644
--- a/net/bpf/bpf_dummy_struct_ops.c
+++ b/net/bpf/bpf_dummy_struct_ops.c
@@ -133,7 +133,9 @@  int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
 	if (err < 0)
 		goto out;
 
-	arch_protect_bpf_trampoline(image, PAGE_SIZE);
+	err = arch_protect_bpf_trampoline(image, PAGE_SIZE);
+	if (err)
+		goto out;
 	prog_ret = dummy_ops_call_op(image, args);
 
 	err = dummy_ops_copy_args(args);