Re: [PATCH v2 2/3] libsepol: avoid implicit conversions

[Nicolas Iooss <nicolas.iooss@m4x.org>]

On Mon, Jul 12, 2021 at 9:36 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Thu, Jul 1, 2021 at 8:06 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Avoid implicit conversions from signed to unsigned values, found by
> > UB sanitizers, by using unsigned values in the first place.
> >
> >     expand.c:1644:18: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)
> >
> >     expand.c:2892:24: runtime error: implicit conversion from type 'int' of value -2 (32-bit, signed) to type 'unsigned int' changed the value to 4294967294 (32-bit, unsigned)
> >
> >     policy_define.c:2344:4: runtime error: implicit conversion from type 'int' of value -1048577 (32-bit, signed) to type 'unsigned int' changed the value to 4293918719 (32-bit, unsigned)
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> Thanks,
> Nicolas

Merged.
Thanks!
Nicolas

> > ---
> > v2:
> >   - use UINT32_C as suggested by Ondrej Mosnacek
> >
> >  libsepol/include/sepol/policydb/conditional.h | 2 +-
> >  libsepol/include/sepol/policydb/policydb.h    | 6 +++---
> >  libsepol/src/expand.c                         | 2 +-
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/libsepol/include/sepol/policydb/conditional.h b/libsepol/include/sepol/policydb/conditional.h
> > index 9c3df3ef..49c0d766 100644
> > --- a/libsepol/include/sepol/policydb/conditional.h
> > +++ b/libsepol/include/sepol/policydb/conditional.h
> > @@ -90,7 +90,7 @@ typedef struct cond_node {
> >         uint32_t expr_pre_comp;
> >         struct cond_node *next;
> >         /* a tunable conditional, calculated and used at expansion */
> > -#define        COND_NODE_FLAGS_TUNABLE 0x01
> > +#define        COND_NODE_FLAGS_TUNABLE UINT32_C(0x01)
> >         uint32_t flags;
> >  } cond_node_t;
> >
> > diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
> > index 6976ef48..4bf9f05d 100644
> > --- a/libsepol/include/sepol/policydb/policydb.h
> > +++ b/libsepol/include/sepol/policydb/policydb.h
> > @@ -251,9 +251,9 @@ typedef struct class_perm_node {
> >         struct class_perm_node *next;
> >  } class_perm_node_t;
> >
> > -#define xperm_test(x, p) (1 & (p[x >> 5] >> (x & 0x1f)))
> > -#define xperm_set(x, p) (p[x >> 5] |= (1 << (x & 0x1f)))
> > -#define xperm_clear(x, p) (p[x >> 5] &= ~(1 << (x & 0x1f)))
> > +#define xperm_test(x, p) (UINT32_C(1) & (p[x >> 5] >> (x & 0x1f)))
> > +#define xperm_set(x, p) (p[x >> 5] |= (UINT32_C(1) << (x & 0x1f)))
> > +#define xperm_clear(x, p) (p[x >> 5] &= ~(UINT32_C(1) << (x & 0x1f)))
> >  #define EXTENDED_PERMS_LEN 8
> >
> >  typedef struct av_extended_perms {
> > diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> > index 84bfcfa3..aac5b35f 100644
> > --- a/libsepol/src/expand.c
> > +++ b/libsepol/src/expand.c
> > @@ -1641,7 +1641,7 @@ static avtab_ptr_t find_avtab_node(sepol_handle_t * handle,
> >                  * AUDITDENY, aka DONTAUDIT, are &= assigned, versus |= for
> >                  * others. Initialize the data accordingly.
> >                  */
> > -               avdatum.data = key->specified == AVTAB_AUDITDENY ? ~0 : 0;
> > +               avdatum.data = key->specified == AVTAB_AUDITDENY ? ~UINT32_C(0) : UINT32_C(0);
> >                 /* this is used to get the node - insertion is actually unique */
> >                 node = avtab_insert_nonunique(avtab, key, &avdatum);
> >                 if (!node) {
> > --
> > 2.32.0
> >