On 08/06/2023 09:42, Zachary Leaf wrote:
union bpf_attr member info.info is a __user *ptr to either: struct bpf_btf_info struct bpf_prog_info struct bpf_map_info struct bpf_link_info
These info structs are passed in from userspace and used to store information returned from calls to BPF_OBJ_GET_INFO_BY_FD.
While inbound conversion for all other members of union bpf_attr is handled upfront, this same approach is not possible here for two main reasons:
Due to the extra complexity of a further userspace struct that is one of 4 types. These types themselves can be a union/multiplex of even more additional options and types. It is not straight forward or possible in many cases to determine which elements of the union are active upfront, and each has different native/compat64 layouts/offsets.
Due to the nature of this subcommand, the main purpose is to copy information back out to userspace, there is more work to do to convert the struct back out to compat64 compatible offsets compared to other subcommands.
Instead of an upfront conversion, convert where each is used at the end site both in/out.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com
include/linux/bpf_compat.h | 19 ++++++++++ kernel/bpf/syscall.c | 75 +++++++++++++++++++++++++++++++++++--- 2 files changed, 88 insertions(+), 6 deletions(-)
diff --git a/include/linux/bpf_compat.h b/include/linux/bpf_compat.h index 34aa707b021e..93a2fcab5746 100644 --- a/include/linux/bpf_compat.h +++ b/include/linux/bpf_compat.h @@ -329,6 +329,25 @@ struct compat_bpf_prog_info { __u32 attach_btf_id; } __attribute__((aligned(8))); +struct compat_bpf_map_info {
- __u32 type;
- __u32 id;
- __u32 key_size;
- __u32 value_size;
- __u32 max_entries;
- __u32 map_flags;
- char name[BPF_OBJ_NAME_LEN];
- __u32 ifindex;
- __u32 btf_vmlinux_value_type_id;
- __u64 netns_dev;
- __u64 netns_ino;
- __u32 btf_id;
- __u32 btf_key_type_id;
- __u32 btf_value_type_id;
- __u32:32; /* alignment pad */
- __u64 map_extra;
+} __attribute__((aligned(8)));
I've realised this entire patch should be able to be skipped. Since the struct doesn't contain any pointers, the size should be the same in native PCuABI/compat64 and no conversion is required.
struct compat_bpf_btf_info { __aligned_u64 btf; __u32 btf_size; diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 929b515c56ae..588e4d5d4077 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -4314,20 +4314,74 @@ static int bpf_prog_get_info_by_fd(struct file *file, return 0; } +#ifdef CONFIG_COMPAT64 +void convert_compat_map_info_out(struct compat_bpf_map_info *dest,
struct bpf_map_info *info)+{
- dest->type = info->type;
- dest->id = info->id;
- dest->key_size = info->key_size;
- dest->value_size = info->value_size;
- dest->max_entries = info->max_entries;
- dest->map_flags = info->map_flags;
- strncpy((char *)dest->name, (char *)info->name, BPF_OBJ_NAME_LEN);
- dest->ifindex = info->ifindex;
- dest->btf_vmlinux_value_type_id = info->btf_vmlinux_value_type_id;
- dest->netns_dev = info->netns_dev;
- dest->netns_ino = info->netns_ino;
- dest->btf_id = info->btf_id;
- dest->btf_key_type_id = info->btf_key_type_id;
- dest->btf_value_type_id = info->btf_value_type_id;
- dest->map_extra = info->map_extra;
+} +#endif /* CONFIG_COMPAT64 */
static int bpf_map_get_info_by_fd(struct file *file, struct bpf_map *map, const union bpf_attr *attr, union bpf_attr __user *uattr) {
- struct bpf_map_info __user *uinfo = u64_to_user_ptr(attr->info.info);
- struct bpf_map_info __user *uinfo; struct bpf_map_info info;
- u32 info_len = attr->info.info_len;
- u32 info_len; int err;
+#ifdef CONFIG_COMPAT64
- struct compat_bpf_map_info __user *cuinfo;
- struct compat_bpf_map_info cinfo;
- union compat_bpf_attr __user *cuattr =
(union compat_bpf_attr __user *)uattr;- u32 cuinfo_len;
- /*
* for compat64, uattr has already been converted and copied into* attr - however attr->info.info is a ptr to uapi struct bpf_map_info** bpf_map_get_info_by_fd() does not use any info from the* bpf_map_info struct passed in from userspace, so only need to* convert to compat64 struct layout when writing back out to userspace*/- if (in_compat_syscall()) {
cuinfo = (struct compat_bpf_map_info __user *)attr->info.info;cuinfo_len = attr->info.info_len;
- err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo), sizeof(info), info_len);
- if (err)
return err;- info_len = min_t(u32, sizeof(info), info_len);
err = bpf_check_uarg_tail_zero(USER_BPFPTR(cuinfo),sizeof(cinfo), cuinfo_len);if (err)return err;cuinfo_len = min_t(u32, sizeof(cinfo), cuinfo_len);- } else
+#endif
- {
uinfo = u64_to_user_ptr(attr->info.info);info_len = attr->info.info_len;err = bpf_check_uarg_tail_zero(USER_BPFPTR(uinfo),sizeof(info), info_len);if (err)return err;info_len = min_t(u32, sizeof(info), info_len);- }
memset(&info, 0, sizeof(info)); info.type = map->map_type; @@ -4352,6 +4406,15 @@ static int bpf_map_get_info_by_fd(struct file *file, return err; } +#ifdef CONFIG_COMPAT64
- if (in_compat_syscall()) {
convert_compat_map_info_out(&cinfo, &info);if (copy_to_user(cuinfo, &cinfo, cuinfo_len) ||put_user(cuinfo_len, &cuattr->info.info_len))return -EFAULT;return 0;- }
+#endif if (copy_to_user(uinfo, &info, info_len) || put_user(info_len, &uattr->info.info_len)) return -EFAULT;