| Message ID | 20211124002325.1737739-4-andrii@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Fix sanitizer-reported libbpf and selftest issues | expand |
On 11/24/21 1:23 AM, Andrii Nakryiko wrote: > Integer overflow is intentional, silence the sanitizer. It works > completely reliably on sane compilers and architectures. > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > --- > tools/lib/bpf/btf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index 8024fe355ca8..be1dafd56a13 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -3127,6 +3127,7 @@ struct btf_dedup { > struct strset *strs_set; > }; > > +__attribute__((no_sanitize("signed-integer-overflow"))) > static long hash_combine(long h, long value) > { > return h * 31 + value; > Sgtm, I guess my only question, was there a reason for not using e.g. __u64 in the first place? Meaning, __u64 hash_combine(__u64 h, __u64 value) plus the call-sites where you have h variable re-feeding into hash_combine().
On 11/25/21 11:21 PM, Daniel Borkmann wrote: > On 11/24/21 1:23 AM, Andrii Nakryiko wrote: >> Integer overflow is intentional, silence the sanitizer. It works >> completely reliably on sane compilers and architectures. >> >> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> >> --- >> tools/lib/bpf/btf.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c >> index 8024fe355ca8..be1dafd56a13 100644 >> --- a/tools/lib/bpf/btf.c >> +++ b/tools/lib/bpf/btf.c >> @@ -3127,6 +3127,7 @@ struct btf_dedup { >> struct strset *strs_set; >> }; >> +__attribute__((no_sanitize("signed-integer-overflow"))) >> static long hash_combine(long h, long value) >> { >> return h * 31 + value; >> > > Sgtm, I guess my only question, was there a reason for not using e.g. __u64 in > the first place? Meaning, __u64 hash_combine(__u64 h, __u64 value) plus the > call-sites where you have h variable re-feeding into hash_combine(). Given the remainder of the series is all straight forward, I took that in already, but would still be nice if we can silence the sanitizer complaint w/o such attribute workaround. Thanks, Daniel
On Thu, Nov 25, 2021 at 3:19 PM Daniel Borkmann <daniel@iogearbox.net> wrote: > > On 11/25/21 11:21 PM, Daniel Borkmann wrote: > > On 11/24/21 1:23 AM, Andrii Nakryiko wrote: > >> Integer overflow is intentional, silence the sanitizer. It works > >> completely reliably on sane compilers and architectures. > >> > >> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > >> --- > >> tools/lib/bpf/btf.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > >> index 8024fe355ca8..be1dafd56a13 100644 > >> --- a/tools/lib/bpf/btf.c > >> +++ b/tools/lib/bpf/btf.c > >> @@ -3127,6 +3127,7 @@ struct btf_dedup { > >> struct strset *strs_set; > >> }; > >> +__attribute__((no_sanitize("signed-integer-overflow"))) > >> static long hash_combine(long h, long value) > >> { > >> return h * 31 + value; > >> > > > > Sgtm, I guess my only question, was there a reason for not using e.g. __u64 in > > the first place? Meaning, __u64 hash_combine(__u64 h, __u64 value) plus the > > call-sites where you have h variable re-feeding into hash_combine(). > > Given the remainder of the series is all straight forward, I took that in already, > but would still be nice if we can silence the sanitizer complaint w/o such attribute > workaround. You are right, I'll follow up with u64 conversion and will drop the attribute. > > Thanks, > Daniel
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c index 8024fe355ca8..be1dafd56a13 100644 --- a/tools/lib/bpf/btf.c +++ b/tools/lib/bpf/btf.c @@ -3127,6 +3127,7 @@ struct btf_dedup { struct strset *strs_set; }; +__attribute__((no_sanitize("signed-integer-overflow"))) static long hash_combine(long h, long value) { return h * 31 + value;
Integer overflow is intentional, silence the sanitizer. It works completely reliably on sane compilers and architectures. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/lib/bpf/btf.c | 1 + 1 file changed, 1 insertion(+)