Merge branch 'bpf-fix-memory-access-flags-in-helper-prototypes'

Zesen Liu says:

====================
bpf: Fix memory access flags in helper prototypes

This series adds missing memory access flags (MEM_RDONLY or MEM_WRITE) to
several bpf helper function prototypes that use ARG_PTR_TO_MEM but lack the
correct flag. It also adds a new check in verifier to ensure the flag is
specified.

Missing memory access flags in helper prototypes can lead to critical
correctness issues when the verifier tries to perform code optimization.
After commit 37cce22dbd51 ("bpf: verifier: Refactor helper access type
tracking"), the verifier relies on the memory access flags, rather than
treating all arguments in helper functions as potentially modifying the
pointed-to memory.

Using ARG_PTR_TO_MEM alone without flags does not make sense because:

- If the helper does not change the argument, missing MEM_RDONLY causes the
verifier to incorrectly reject a read-only buffer.
- If the helper does change the argument, missing MEM_WRITE causes the
verifier to incorrectly assume the memory is unchanged, leading to
errors in code optimization.

We have already seen several reports regarding this:

- commit ac44dcc788b9 ("bpf: Fix verifier assumptions of bpf_d_path's
output buffer") adds MEM_WRITE to bpf_d_path;
- commit 2eb7648558a7 ("bpf: Specify access type of bpf_sysctl_get_name
args") adds MEM_WRITE to bpf_sysctl_get_name.

This series looks through all prototypes in the kernel and completes the
flags. It also adds check_mem_arg_rw_flag_ok() and wires it into
check_func_proto() to statically restrict ARG_PTR_TO_MEM from appearing
without memory access flags.

Changelog
=========

v3:
- Rebased to bpf-next to address check_func_proto() signature changes, as
suggested by Eduard Zingerman.

v2:
- Add missing MEM_RDONLY flags to protos with ARG_PTR_TO_FIXED_SIZE_MEM.

====================

Link: https://patch.msgid.link/20260120-helper_proto-v3-0-27b0180b4e77@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Alexei Starovoitov 4 months ago 2516a9c5 900dbb6d

+32 -15

5 changed files

expand all

kernel

bpf

helpers.c

syscall.c

verifier.c

trace

bpf_trace.c

net

core

filter.c

+1 -1

kernel/bpf/helpers.c

··· 1077 1077 .func = bpf_snprintf, 1078 1078 .gpl_only = true, 1079 1079 .ret_type = RET_INTEGER, 1080 - .arg1_type = ARG_PTR_TO_MEM_OR_NULL, 1080 + .arg1_type = ARG_PTR_TO_MEM_OR_NULL | MEM_WRITE, 1081 1081 .arg2_type = ARG_CONST_SIZE_OR_ZERO, 1082 1082 .arg3_type = ARG_PTR_TO_CONST_STR, 1083 1083 .arg4_type = ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY,

+1 -1

kernel/bpf/syscall.c

··· 6451 6451 .func = bpf_kallsyms_lookup_name, 6452 6452 .gpl_only = false, 6453 6453 .ret_type = RET_INTEGER, 6454 - .arg1_type = ARG_PTR_TO_MEM, 6454 + .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY, 6455 6455 .arg2_type = ARG_CONST_SIZE_OR_ZERO, 6456 6456 .arg3_type = ARG_ANYTHING, 6457 6457 .arg4_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_WRITE | MEM_ALIGNED,

+17

kernel/bpf/verifier.c

··· 10441 10441 return true; 10442 10442 } 10443 10443 10444 + static bool check_mem_arg_rw_flag_ok(const struct bpf_func_proto *fn) 10445 + { 10446 + int i; 10447 + 10448 + for (i = 0; i < ARRAY_SIZE(fn->arg_type); i++) { 10449 + enum bpf_arg_type arg_type = fn->arg_type[i]; 10450 + 10451 + if (base_type(arg_type) != ARG_PTR_TO_MEM) 10452 + continue; 10453 + if (!(arg_type & (MEM_WRITE | MEM_RDONLY))) 10454 + return false; 10455 + } 10456 + 10457 + return true; 10458 + } 10459 + 10444 10460 static int check_func_proto(const struct bpf_func_proto *fn) 10445 10461 { 10446 10462 return check_raw_mode_ok(fn) && 10447 10463 check_arg_pair_ok(fn) && 10464 + check_mem_arg_rw_flag_ok(fn) && 10448 10465 check_btf_id_ok(fn) ? 0 : -EINVAL; 10449 10466 } 10450 10467

+3 -3

kernel/trace/bpf_trace.c

··· 1022 1022 .func = bpf_snprintf_btf, 1023 1023 .gpl_only = false, 1024 1024 .ret_type = RET_INTEGER, 1025 - .arg1_type = ARG_PTR_TO_MEM, 1025 + .arg1_type = ARG_PTR_TO_MEM | MEM_WRITE, 1026 1026 .arg2_type = ARG_CONST_SIZE, 1027 1027 .arg3_type = ARG_PTR_TO_MEM | MEM_RDONLY, 1028 1028 .arg4_type = ARG_CONST_SIZE, ··· 1526 1526 .gpl_only = true, 1527 1527 .ret_type = RET_INTEGER, 1528 1528 .arg1_type = ARG_PTR_TO_CTX, 1529 - .arg2_type = ARG_PTR_TO_MEM_OR_NULL, 1529 + .arg2_type = ARG_PTR_TO_MEM_OR_NULL | MEM_WRITE, 1530 1530 .arg3_type = ARG_CONST_SIZE_OR_ZERO, 1531 1531 .arg4_type = ARG_ANYTHING, 1532 1532 }; ··· 1661 1661 .gpl_only = true, 1662 1662 .ret_type = RET_INTEGER, 1663 1663 .arg1_type = ARG_PTR_TO_CTX, 1664 - .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, 1664 + .arg2_type = ARG_PTR_TO_UNINIT_MEM, 1665 1665 .arg3_type = ARG_CONST_SIZE_OR_ZERO, 1666 1666 .arg4_type = ARG_ANYTHING, 1667 1667 };

+10 -10

net/core/filter.c

··· 6399 6399 .gpl_only = true, 6400 6400 .ret_type = RET_INTEGER, 6401 6401 .arg1_type = ARG_PTR_TO_CTX, 6402 - .arg2_type = ARG_PTR_TO_MEM, 6402 + .arg2_type = ARG_PTR_TO_MEM | MEM_WRITE, 6403 6403 .arg3_type = ARG_CONST_SIZE, 6404 6404 .arg4_type = ARG_ANYTHING, 6405 6405 }; ··· 6454 6454 .gpl_only = true, 6455 6455 .ret_type = RET_INTEGER, 6456 6456 .arg1_type = ARG_PTR_TO_CTX, 6457 - .arg2_type = ARG_PTR_TO_MEM, 6457 + .arg2_type = ARG_PTR_TO_MEM | MEM_WRITE, 6458 6458 .arg3_type = ARG_CONST_SIZE, 6459 6459 .arg4_type = ARG_ANYTHING, 6460 6460 }; ··· 8008 8008 .gpl_only = true, /* __cookie_v4_init_sequence() is GPL */ 8009 8009 .pkt_access = true, 8010 8010 .ret_type = RET_INTEGER, 8011 - .arg1_type = ARG_PTR_TO_FIXED_SIZE_MEM, 8011 + .arg1_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_RDONLY, 8012 8012 .arg1_size = sizeof(struct iphdr), 8013 - .arg2_type = ARG_PTR_TO_MEM, 8013 + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, 8014 8014 .arg3_type = ARG_CONST_SIZE_OR_ZERO, 8015 8015 }; 8016 8016 ··· 8040 8040 .gpl_only = true, /* __cookie_v6_init_sequence() is GPL */ 8041 8041 .pkt_access = true, 8042 8042 .ret_type = RET_INTEGER, 8043 - .arg1_type = ARG_PTR_TO_FIXED_SIZE_MEM, 8043 + .arg1_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_RDONLY, 8044 8044 .arg1_size = sizeof(struct ipv6hdr), 8045 - .arg2_type = ARG_PTR_TO_MEM, 8045 + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, 8046 8046 .arg3_type = ARG_CONST_SIZE_OR_ZERO, 8047 8047 }; 8048 8048 ··· 8060 8060 .gpl_only = true, /* __cookie_v4_check is GPL */ 8061 8061 .pkt_access = true, 8062 8062 .ret_type = RET_INTEGER, 8063 - .arg1_type = ARG_PTR_TO_FIXED_SIZE_MEM, 8063 + .arg1_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_RDONLY, 8064 8064 .arg1_size = sizeof(struct iphdr), 8065 - .arg2_type = ARG_PTR_TO_FIXED_SIZE_MEM, 8065 + .arg2_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_RDONLY, 8066 8066 .arg2_size = sizeof(struct tcphdr), 8067 8067 }; 8068 8068 ··· 8084 8084 .gpl_only = true, /* __cookie_v6_check is GPL */ 8085 8085 .pkt_access = true, 8086 8086 .ret_type = RET_INTEGER, 8087 - .arg1_type = ARG_PTR_TO_FIXED_SIZE_MEM, 8087 + .arg1_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_RDONLY, 8088 8088 .arg1_size = sizeof(struct ipv6hdr), 8089 - .arg2_type = ARG_PTR_TO_FIXED_SIZE_MEM, 8089 + .arg2_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_RDONLY, 8090 8090 .arg2_size = sizeof(struct tcphdr), 8091 8091 }; 8092 8092 #endif /* CONFIG_SYN_COOKIES */

Configure Feed

Configure Feed