Message ID | 159293239241.32225.12338844121877017327.stgit@john-Precision-5820-Tower |
---|---|
State | Changes Requested |
Delegated to: | BPF Maintainers |
Headers | show |
Series | [bpf] bpf: do not allow btf_ctx_access with __int128 types | expand |
On Tue, Jun 23, 2020 at 10:14 AM John Fastabend <john.fastabend@gmail.com> wrote: > > To ensure btf_ctx_access() is safe the verifier checks that the BTF > arg type is an int, enum, or pointer. When the function does the > BTF arg lookup it uses the calculation 'arg = off / 8' using the > fact that registers are 8B. This requires that the first arg is > in the first reg, the second in the second, and so on. However, > for __int128 the arg will consume two registers by default LLVM > implementation. So this will cause the arg layout assumed by the > 'arg = off / 8' calculation to be incorrect. > > Because __int128 is uncommon this patch applies the easiest fix and > will force int types to be sizeof(u64) or smaller so that they will > fit in a single register. > > Fixes: 9e15db66136a1 ("bpf: Implement accurate raw_tp context access via BTF") > Signed-off-by: John Fastabend <john.fastabend@gmail.com> > --- "small int" for u64 looks funny, but naming is hard :) Acked-by: Andrii Nakryiko <andriin@fb.com> > include/linux/btf.h | 5 +++++ > kernel/bpf/btf.c | 4 ++-- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/include/linux/btf.h b/include/linux/btf.h > index 5c1ea99..35642f6 100644 > --- a/include/linux/btf.h > +++ b/include/linux/btf.h > @@ -82,6 +82,11 @@ static inline bool btf_type_is_int(const struct btf_type *t) > return BTF_INFO_KIND(t->info) == BTF_KIND_INT; > } > > +static inline bool btf_type_is_small_int(const struct btf_type *t) > +{ > + return btf_type_is_int(t) && (t->size <= sizeof(u64)); nit: unnecessary (), () are usually used to disambiguate | and & vs || and &&; this is not the case, though. > +} > + > static inline bool btf_type_is_enum(const struct btf_type *t) > { > return BTF_INFO_KIND(t->info) == BTF_KIND_ENUM; > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 58c9af1..9a1a98d 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -3746,7 +3746,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, > return false; > > t = btf_type_skip_modifiers(btf, t->type, NULL); > - if (!btf_type_is_int(t)) { > + if (!btf_type_is_small_int(t)) { > bpf_log(log, > "ret type %s not allowed for fmod_ret\n", > btf_kind_str[BTF_INFO_KIND(t->info)]); > @@ -3768,7 +3768,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, > /* skip modifiers */ > while (btf_type_is_modifier(t)) > t = btf_type_by_id(btf, t->type); > - if (btf_type_is_int(t) || btf_type_is_enum(t)) > + if (btf_type_is_small_int(t) || btf_type_is_enum(t)) > /* accessing a scalar */ > return true; > if (!btf_type_is_ptr(t)) { >
Andrii Nakryiko wrote: > On Tue, Jun 23, 2020 at 10:14 AM John Fastabend > <john.fastabend@gmail.com> wrote: > > > > To ensure btf_ctx_access() is safe the verifier checks that the BTF > > arg type is an int, enum, or pointer. When the function does the > > BTF arg lookup it uses the calculation 'arg = off / 8' using the > > fact that registers are 8B. This requires that the first arg is > > in the first reg, the second in the second, and so on. However, > > for __int128 the arg will consume two registers by default LLVM > > implementation. So this will cause the arg layout assumed by the > > 'arg = off / 8' calculation to be incorrect. > > > > Because __int128 is uncommon this patch applies the easiest fix and > > will force int types to be sizeof(u64) or smaller so that they will > > fit in a single register. > > > > Fixes: 9e15db66136a1 ("bpf: Implement accurate raw_tp context access via BTF") > > Signed-off-by: John Fastabend <john.fastabend@gmail.com> > > --- > > "small int" for u64 looks funny, but naming is hard :) > > Acked-by: Andrii Nakryiko <andriin@fb.com> > Fixed up the parens and sent a v2 keeping the ACK. I don't have any better ideas for the name, let me know if you have a preference for something else.
diff --git a/include/linux/btf.h b/include/linux/btf.h index 5c1ea99..35642f6 100644 --- a/include/linux/btf.h +++ b/include/linux/btf.h @@ -82,6 +82,11 @@ static inline bool btf_type_is_int(const struct btf_type *t) return BTF_INFO_KIND(t->info) == BTF_KIND_INT; } +static inline bool btf_type_is_small_int(const struct btf_type *t) +{ + return btf_type_is_int(t) && (t->size <= sizeof(u64)); +} + static inline bool btf_type_is_enum(const struct btf_type *t) { return BTF_INFO_KIND(t->info) == BTF_KIND_ENUM; diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 58c9af1..9a1a98d 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -3746,7 +3746,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, return false; t = btf_type_skip_modifiers(btf, t->type, NULL); - if (!btf_type_is_int(t)) { + if (!btf_type_is_small_int(t)) { bpf_log(log, "ret type %s not allowed for fmod_ret\n", btf_kind_str[BTF_INFO_KIND(t->info)]); @@ -3768,7 +3768,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, /* skip modifiers */ while (btf_type_is_modifier(t)) t = btf_type_by_id(btf, t->type); - if (btf_type_is_int(t) || btf_type_is_enum(t)) + if (btf_type_is_small_int(t) || btf_type_is_enum(t)) /* accessing a scalar */ return true; if (!btf_type_is_ptr(t)) {
To ensure btf_ctx_access() is safe the verifier checks that the BTF arg type is an int, enum, or pointer. When the function does the BTF arg lookup it uses the calculation 'arg = off / 8' using the fact that registers are 8B. This requires that the first arg is in the first reg, the second in the second, and so on. However, for __int128 the arg will consume two registers by default LLVM implementation. So this will cause the arg layout assumed by the 'arg = off / 8' calculation to be incorrect. Because __int128 is uncommon this patch applies the easiest fix and will force int types to be sizeof(u64) or smaller so that they will fit in a single register. Fixes: 9e15db66136a1 ("bpf: Implement accurate raw_tp context access via BTF") Signed-off-by: John Fastabend <john.fastabend@gmail.com> --- include/linux/btf.h | 5 +++++ kernel/bpf/btf.c | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-)