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 > --- > 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 >
