On Tue, Jun 8, 2021 at 6:01 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.
>
> 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'
>
> Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> ---
> libsepol/src/avtab.c | 6 ++++++
> libsepol/src/policydb.c | 6 ++++++
> libsepol/src/symtab.c | 6 ++++++
> 3 files changed, 18 insertions(+)
>
> diff --git a/libsepol/src/avtab.c b/libsepol/src/avtab.c
> index 257f051a..c2ccb005 100644
> --- a/libsepol/src/avtab.c
> +++ b/libsepol/src/avtab.c
> @@ -52,6 +52,12 @@
> /* Based on MurmurHash3, written by Austin Appleby and placed in the
> * public domain.
> */
> +#if defined(__clang__) && defined(__clang_major__) && (__clang_major__ >= 4)
> +__attribute__((no_sanitize("unsigned-integer-overflow")))
> +#if (__clang_major__ >= 12)
> +__attribute__((no_sanitize("unsigned-shift-base")))
> +#endif
> +#endif
Perhaps this could be defined as a common macro in
libsepol/src/private.h instead of duplicating it in three places?
> 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 fc1d0711..cbe0c432 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -789,6 +789,12 @@ static int roles_init(policydb_t * p)
> goto out;
> }
>
> +#if defined(__clang__) && defined(__clang_major__) && (__clang_major__ >= 4)
> +__attribute__((no_sanitize("unsigned-integer-overflow")))
> +#if (__clang_major__ >= 12)
> +__attribute__((no_sanitize("unsigned-shift-base")))
> +#endif
> +#endif
> static inline unsigned long
> partial_name_hash(unsigned long c, unsigned long prevhash)
> {
> diff --git a/libsepol/src/symtab.c b/libsepol/src/symtab.c
> index 9a417ca2..738fa0a4 100644
> --- a/libsepol/src/symtab.c
> +++ b/libsepol/src/symtab.c
> @@ -11,6 +11,12 @@
> #include <sepol/policydb/hashtab.h>
> #include <sepol/policydb/symtab.h>
>
> +#if defined(__clang__) && defined(__clang_major__) && (__clang_major__ >= 4)
> +__attribute__((no_sanitize("unsigned-integer-overflow")))
> +#if (__clang_major__ >= 12)
> +__attribute__((no_sanitize("unsigned-shift-base")))
> +#endif
> +#endif
> static unsigned int symhash(hashtab_t h, const_hashtab_key_t key)
> {
> const char *p, *keyp;
> --
> 2.32.0
>
--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
