On Tue, Jul 6, 2021 at 7:36 PM Christian Göttsche
<cgzones@xxxxxxxxxxxxxx> wrote:
>
> Unsigned integer overflow is well-defined and not undefined behavior.
> It is commonly used for hashing or pseudo random number generation.
> 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 or missed overflow checks on user input.
>
> Use a spaceship operator like comparison instead of subtraction.
>
> policydb.c:851:24: runtime error: unsigned integer overflow: 801 - 929 cannot be represented in type 'unsigned int'
>
> Follow-up of: 1537ea8412e4 ("libsepol: avoid unsigned integer overflow")
>
> Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
Acked-by: Nicolas Iooss <nicolas.iooss@xxxxxxx>
Thanks!
Nicolas
> ---
> libsepol/src/policydb.c | 10 +++++-----
> libsepol/src/private.h | 2 ++
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index ef2217c2..0398ceed 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -817,11 +817,11 @@ static int filenametr_cmp(hashtab_t h __attribute__ ((unused)),
> const filename_trans_key_t *ft2 = (const filename_trans_key_t *)k2;
> int v;
>
> - v = (ft1->ttype > ft2->ttype) - (ft1->ttype < ft2->ttype);
> + v = spaceship_cmp(ft1->ttype, ft2->ttype);
> if (v)
> return v;
>
> - v = (ft1->tclass > ft2->tclass) - (ft1->tclass < ft2->tclass);
> + v = spaceship_cmp(ft1->tclass, ft2->tclass);
> if (v)
> return v;
>
> @@ -843,15 +843,15 @@ static int rangetr_cmp(hashtab_t h __attribute__ ((unused)),
> const struct range_trans *key2 = (const struct range_trans *)k2;
> int v;
>
> - v = key1->source_type - key2->source_type;
> + v = spaceship_cmp(key1->source_type, key2->source_type);
> if (v)
> return v;
>
> - v = key1->target_type - key2->target_type;
> + v = spaceship_cmp(key1->target_type, key2->target_type);
> if (v)
> return v;
>
> - v = key1->target_class - key2->target_class;
> + v = spaceship_cmp(key1->target_class, key2->target_class);
>
> return v;
> }
> diff --git a/libsepol/src/private.h b/libsepol/src/private.h
> index 72f21262..c63238ab 100644
> --- a/libsepol/src/private.h
> +++ b/libsepol/src/private.h
> @@ -47,6 +47,8 @@
> #define is_saturated(x) (x == (typeof(x))-1)
> #define zero_or_saturated(x) ((x == 0) || is_saturated(x))
>
> +#define spaceship_cmp(a, b) (((a) > (b)) - ((a) < (b)))
> +
> /* Policy compatibility information. */
> struct policydb_compat_info {
> unsigned int type;
> --
> 2.32.0
>
