| Message ID | 20210701180645.120082-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/3] libsepol: ignore UBSAN false-positives | expand |
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 > --- > 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 >
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 > >
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) {
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> --- 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(-)