On Mon, Jul 12, 2021 at 9:33 AM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote:
>
> 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
Merged.
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
> >
