On Mon, Jul 12, 2021 at 9:34 AM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote: > > On Thu, Jul 1, 2021 at 8:06 PM Christian Göttsche > <cgzones@xxxxxxxxxxxxxx> wrote: > > > > Unsigned integer overflow is well-defined and not undefined behavior. > > But it is still useful to enable undefined behavior sanitizer checks on > > unsigned arithmetic to detect possible issues on counters or variables > > with similar purpose. > > > > Annotate functions, in which unsigned overflows are expected to happen, > > with the respective Clang function attribute[1]. > > GCC does not support sanitizing unsigned integer arithmetic[2]. > > > > avtab.c:76:2: runtime error: unsigned integer overflow: 6 * 3432918353 cannot be represented in type 'unsigned int' > > policydb.c:795:42: runtime error: unsigned integer overflow: 8160943042179512010 * 11 cannot be represented in type 'unsigned long' > > symtab.c:25:12: runtime error: left shift of 1766601759 by 4 places cannot be represented in type 'unsigned int' > > > > [1]: https://clang.llvm.org/docs/AttributeReference.html#no-sanitize > > [2]: https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html > > > > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx> > > Acked-by: Nicolas Iooss <nicolas.iooss@xxxxxxx> > > Thanks, > Nicolas Merged. Thanks! Nicolas > > --- > > v2: > > - use a common macro as suggested by Ondrej Mosnacek > > - mention that GCC does not support unsigned integer sanitation > > > > libsepol/src/avtab.c | 1 + > > libsepol/src/policydb.c | 1 + > > libsepol/src/private.h | 11 +++++++++++ > > libsepol/src/symtab.c | 4 ++++ > > 4 files changed, 17 insertions(+) > > > > diff --git a/libsepol/src/avtab.c b/libsepol/src/avtab.c > > index 88e9d510..37e8af07 100644 > > --- a/libsepol/src/avtab.c > > +++ b/libsepol/src/avtab.c > > @@ -52,6 +52,7 @@ > > /* Based on MurmurHash3, written by Austin Appleby and placed in the > > * public domain. > > */ > > +ignore_unsigned_overflow_ > > static inline int avtab_hash(struct avtab_key *keyp, uint32_t mask) > > { > > static const uint32_t c1 = 0xcc9e2d51; > > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c > > index ef2217c2..0721c81e 100644 > > --- a/libsepol/src/policydb.c > > +++ b/libsepol/src/policydb.c > > @@ -789,6 +789,7 @@ static int roles_init(policydb_t * p) > > goto out; > > } > > > > +ignore_unsigned_overflow_ > > static inline unsigned long > > partial_name_hash(unsigned long c, unsigned long prevhash) > > { > > diff --git a/libsepol/src/private.h b/libsepol/src/private.h > > index 72f21262..e54cefdb 100644 > > --- a/libsepol/src/private.h > > +++ b/libsepol/src/private.h > > @@ -47,6 +47,17 @@ > > #define is_saturated(x) (x == (typeof(x))-1) > > #define zero_or_saturated(x) ((x == 0) || is_saturated(x)) > > > > +/* Use to ignore intentional unsigned under- and overflows while running under UBSAN. */ > > +#if defined(__clang__) && defined(__clang_major__) && (__clang_major__ >= 4) > > +#if (__clang_major__ >= 12) > > +#define ignore_unsigned_overflow_ __attribute__((no_sanitize("unsigned-integer-overflow", "unsigned-shift-base"))) > > +#else > > +#define ignore_unsigned_overflow_ __attribute__((no_sanitize("unsigned-integer-overflow"))) > > +#endif > > +#else > > +#define ignore_unsigned_overflow_ > > +#endif > > + > > /* Policy compatibility information. */ > > struct policydb_compat_info { > > unsigned int type; > > diff --git a/libsepol/src/symtab.c b/libsepol/src/symtab.c > > index 9a417ca2..a6061851 100644 > > --- a/libsepol/src/symtab.c > > +++ b/libsepol/src/symtab.c > > @@ -8,9 +8,13 @@ > > */ > > > > #include <string.h> > > + > > +#include "private.h" > > + > > #include <sepol/policydb/hashtab.h> > > #include <sepol/policydb/symtab.h> > > > > +ignore_unsigned_overflow_ > > static unsigned int symhash(hashtab_t h, const_hashtab_key_t key) > > { > > const char *p, *keyp; > > -- > > 2.32.0 > >
