On 15/11/2022 10:08, Zachary Leaf wrote:
The CHECK_ATTR macro needs adapting to support union compat_bpf_attr as well as union bpf_attr.
The union bpf_attr contains many structs of various sizes for each of the bpf syscall's multiplexed options. After declaring bpf_attr, we zero the entire memory of the union, then copy_from_user() the bpf_attr config struct passed into the syscall to the new struct.
Since most of the structs do not take up the full memory footprint of the union, we should not have any data past the last struct member for any given option. CHECK_ATTR macro checks there is no data except 0 for the gap between the end of the last member and the end of the union.
Adapt the macro to be able to handle compat calls based on in_compat_syscall(). If in compat, use compat_bpf_attr union with the appropriate offsets.
Since syscall options now take a void* parameter instead of bpf_attr* (to also support compat_bpf_attr*), adapt macro to cast the void* appropriately to select the correct member offsets.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com
kernel/bpf/syscall.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 6e3b567d254a..910544bcae6e 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -735,6 +735,18 @@ int bpf_get_file_flag(int flags) offsetof(union bpf_attr, CMD##_LAST_FIELD) - \ sizeof(attr->CMD##_LAST_FIELD)) != NULL +#define __CHECK_ATTR(CMD, TYPE) \
- (memchr_inv((void *) &(((TYPE *)vattr)->CMD##_LAST_FIELD) + \
sizeof(((TYPE *)vattr)->CMD##_LAST_FIELD), 0, \sizeof(*(TYPE *)vattr) - \offsetof(TYPE, CMD##_LAST_FIELD) - \sizeof(((TYPE *)vattr)->CMD##_LAST_FIELD)) != NULL)+#define CHECK_ATTR_FIXED(CMD) \
- (in_compat_syscall() ? \
__CHECK_ATTR(CMD, union compat_bpf_attr) : \__CHECK_ATTR(CMD, union bpf_attr))/* dst and src must have at least "size" number of bytes.
- Return strlen on success and < 0 on error.
*/ @@ -2253,7 +2265,7 @@ static int bpf_prog_load(void *vattr, bpfptr_t uattr) union bpf_attr compat_attr; #endif
- if (CHECK_ATTR(BPF_PROG_LOAD))
If you go for my recommendation in patch 5 (separate compat handler), then all these CHECK_ATTR() should move to __sys_bpf() and be duplicated in __sys_compat_bpf(). On the plus side, in that case, you don't need conditional code in that macro either - just need to replace union bpf_attr with a macro parameter (or even typeof(*attr)).
Kevin
- if (CHECK_ATTR_FIXED(BPF_PROG_LOAD)) return -EINVAL;
#ifdef CONFIG_COMPAT