Re: [PATCH 04/23] libsepol: ignore UBSAN false-positives

[Ondrej Mosnacek <omosnace@redhat.com>]

On Tue, Jun 8, 2021 at 6:01 PM Christian Göttsche
<cgzones@googlemail.com> 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@googlemail.com>
> ---
>  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.