On Wed, Apr 8, 2020 at 4:26 PM Yonghong Song <yhs@xxxxxx> wrote:
>
> In /proc/net/ipv6_route, we have
> struct fib6_info {
> struct fib6_table *fib6_table;
> ...
> struct fib6_nh fib6_nh[0];
> }
> struct fib6_nh {
> struct fib_nh_common nh_common;
> struct rt6_info **rt6i_pcpu;
> struct rt6_exception_bucket *rt6i_exception_bucket;
> };
> struct fib_nh_common {
> ...
> u8 nhc_gw_family;
> ...
> }
>
> The access:
> struct fib6_nh *fib6_nh = &rt->fib6_nh;
> ... fib6_nh->nh_common.nhc_gw_family ...
>
> This patch ensures such an access is handled properly.
>
> Signed-off-by: Yonghong Song <yhs@xxxxxx>
> ---
> kernel/bpf/btf.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index d65c6912bdaf..89a0d983b169 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -3837,6 +3837,31 @@ int btf_struct_access(struct bpf_verifier_log *log,
> }
>
> if (off + size > t->size) {
> + /* If the last element is a variable size array, we may
> + * need to relax the rule.
> + */
> + struct btf_array *array_elem;
> + u32 vlen = btf_type_vlen(t);
> + u32 last_member_type;
> +
> + member = btf_type_member(t);
> + last_member_type = member[vlen - 1].type;
vlen could be zero, and this will be bad
> + mtype = btf_type_by_id(btf_vmlinux, last_member_type);
might want to strip modifiers here?
> + if (!btf_type_is_array(mtype))
> + goto error;
> +
should probably check that off is >= last_member's offset within a
struct? Otherwise access might be spanning previous field and this
array?
> + array_elem = (struct btf_array *)(mtype + 1);
> + if (array_elem->nelems != 0)
> + goto error;
> +
> + elem_type = btf_type_by_id(btf_vmlinux, array_elem->type);
strip modifiers
> + if (!btf_type_is_struct(elem_type))
> + goto error;
> +
> + off = (off - t->size) % elem_type->size;
I think it will be safer to use field offset, not struct size.
Consider example below
$ cat test-test.c
struct bla {
long a;
int b;
char c[];
};
int main() {
static struct bla *x = 0;
return 0;
}
$ pahole -F btf -C bla test-test.o
struct bla {
long int a; /* 0 8 */
int b; /* 8 4 */
char c[]; /* 12 0 */
/* size: 16, cachelines: 1, members: 3 */
/* padding: 4 */
/* last cacheline: 16 bytes */
};
c is at offset 12, but struct size is 16 due to long alignment. It
could be a 4-byte struct instead of char there.
> + return btf_struct_access(log, elem_type, off, size, atype, next_btf_id);
> +
> +error:
> bpf_log(log, "access beyond struct %s at off %u size %u\n",
> tname, off, size);
> return -EACCES;
> --
> 2.24.1
>
