* [PATCH userspace 1/1] libsepol: initialize s in constraint_expr_eval_reason
@ 2022-06-29 7:20 Nicolas Iooss
2022-06-29 20:06 ` James Carter
0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Iooss @ 2022-06-29 7:20 UTC (permalink / raw)
To: selinux
clang's static analyzer reports that s[0] can be uninitialized when used
in:
sprintf(tmp_buf, "%s %s\n",
xcontext ? "Validatetrans" : "Constraint",
s[0] ? "GRANTED" : "DENIED");
Silence this false-positive issue by making s always initialized.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
libsepol/src/services.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libsepol/src/services.c b/libsepol/src/services.c
index d7510e9dae51..db769cdcfaf9 100644
--- a/libsepol/src/services.c
+++ b/libsepol/src/services.c
@@ -394,7 +394,7 @@ static int constraint_expr_eval_reason(context_struct_t *scontext,
role_datum_t *r1, *r2;
mls_level_t *l1, *l2;
constraint_expr_t *e;
- int s[CEXPR_MAXDEPTH];
+ int s[CEXPR_MAXDEPTH] = {};
int sp = -1;
char tmp_buf[128];
--
2.36.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH userspace 1/1] libsepol: initialize s in constraint_expr_eval_reason
2022-06-29 7:20 [PATCH userspace 1/1] libsepol: initialize s in constraint_expr_eval_reason Nicolas Iooss
@ 2022-06-29 20:06 ` James Carter
2022-07-01 9:21 ` Christian Göttsche
2022-07-06 20:11 ` James Carter
0 siblings, 2 replies; 4+ messages in thread
From: James Carter @ 2022-06-29 20:06 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: SElinux list
On Wed, Jun 29, 2022 at 3:37 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> clang's static analyzer reports that s[0] can be uninitialized when used
> in:
>
> sprintf(tmp_buf, "%s %s\n",
> xcontext ? "Validatetrans" : "Constraint",
> s[0] ? "GRANTED" : "DENIED");
>
> Silence this false-positive issue by making s always initialized.
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: James Carter <jwcart2@gmail.com>
> ---
> libsepol/src/services.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libsepol/src/services.c b/libsepol/src/services.c
> index d7510e9dae51..db769cdcfaf9 100644
> --- a/libsepol/src/services.c
> +++ b/libsepol/src/services.c
> @@ -394,7 +394,7 @@ static int constraint_expr_eval_reason(context_struct_t *scontext,
> role_datum_t *r1, *r2;
> mls_level_t *l1, *l2;
> constraint_expr_t *e;
> - int s[CEXPR_MAXDEPTH];
> + int s[CEXPR_MAXDEPTH] = {};
> int sp = -1;
> char tmp_buf[128];
>
> --
> 2.36.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH userspace 1/1] libsepol: initialize s in constraint_expr_eval_reason
2022-06-29 20:06 ` James Carter
@ 2022-07-01 9:21 ` Christian Göttsche
2022-07-06 20:11 ` James Carter
1 sibling, 0 replies; 4+ messages in thread
From: Christian Göttsche @ 2022-07-01 9:21 UTC (permalink / raw)
To: James Carter; +Cc: Nicolas Iooss, SElinux list
On Wed, 29 Jun 2022 at 22:07, James Carter <jwcart2@gmail.com> wrote:
>
> On Wed, Jun 29, 2022 at 3:37 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> >
> > clang's static analyzer reports that s[0] can be uninitialized when used
> > in:
> >
> > sprintf(tmp_buf, "%s %s\n",
> > xcontext ? "Validatetrans" : "Constraint",
> > s[0] ? "GRANTED" : "DENIED");
> >
The trace for the reports shows:
441 for (e = constraint->expr; e; e = e->next) {
Loop condition is false. Execution continues on line 708
and clang-tidy also reports:
libsepol/src/services.c:715:16: warning: Call to 'calloc' has an
allocation size of 0 bytes [clang-analyzer-optin.portability.UnixAPI]
answer_list = calloc(expr_count, sizeof(*answer_list));
^ ~~~~~~~~~~
libsepol/src/services.c:433:6: note: Assuming 'class_buf' is
non-null
if (!class_buf) {
^~~~~~~~~~
libsepol/src/services.c:433:2: note: Taking false branch
if (!class_buf) {
^
libsepol/src/services.c:439:2: note: The value 0 is assigned to
'expr_counter'
expr_counter = 0;
^~~~~~~~~~~~~~~~
libsepol/src/services.c:441:2: note: Loop condition is false.
Execution continues on line 708
for (e = constraint->expr; e; e = e->next) {
^
libsepol/src/services.c:708:2: note: The value 0 is assigned to
'expr_count'
expr_count = expr_counter;
^~~~~~~~~~~~~~~~~~~~~~~~~
libsepol/src/services.c:715:16: note: Call to 'calloc' has an
allocation size of 0 bytes
answer_list = calloc(expr_count, sizeof(*answer_list));
^ ~~~~~~~~~~
I think the root cause is the possibility of `constraint->expr` being
NULL, i.e. the constraint having no expression, which seems invalid to
me.
Maybe add a check `if(!constraint->expr) { BUG(); return -EINVAL; }`?
Also validation should probably catch this at
https://github.com/SELinuxProject/selinux/blob/956bda08f6183078f13b70f6aa27d0529a3ec20a/libsepol/src/policydb_validate.c#L238
+ if (!cons->expr)
+ goto bad;
+
for (cexp = cons->expr; cexp; cexp = cexp->next) {
> > Silence this false-positive issue by making s always initialized.
> >
> > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> Acked-by: James Carter <jwcart2@gmail.com>
>
> > ---
> > libsepol/src/services.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libsepol/src/services.c b/libsepol/src/services.c
> > index d7510e9dae51..db769cdcfaf9 100644
> > --- a/libsepol/src/services.c
> > +++ b/libsepol/src/services.c
> > @@ -394,7 +394,7 @@ static int constraint_expr_eval_reason(context_struct_t *scontext,
> > role_datum_t *r1, *r2;
> > mls_level_t *l1, *l2;
> > constraint_expr_t *e;
> > - int s[CEXPR_MAXDEPTH];
> > + int s[CEXPR_MAXDEPTH] = {};
> > int sp = -1;
> > char tmp_buf[128];
> >
> > --
> > 2.36.1
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH userspace 1/1] libsepol: initialize s in constraint_expr_eval_reason
2022-06-29 20:06 ` James Carter
2022-07-01 9:21 ` Christian Göttsche
@ 2022-07-06 20:11 ` James Carter
1 sibling, 0 replies; 4+ messages in thread
From: James Carter @ 2022-07-06 20:11 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: SElinux list
On Wed, Jun 29, 2022 at 4:06 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Wed, Jun 29, 2022 at 3:37 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> >
> > clang's static analyzer reports that s[0] can be uninitialized when used
> > in:
> >
> > sprintf(tmp_buf, "%s %s\n",
> > xcontext ? "Validatetrans" : "Constraint",
> > s[0] ? "GRANTED" : "DENIED");
> >
> > Silence this false-positive issue by making s always initialized.
> >
> > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> Acked-by: James Carter <jwcart2@gmail.com>
>
Merged.
Thanks,
Jim
> > ---
> > libsepol/src/services.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libsepol/src/services.c b/libsepol/src/services.c
> > index d7510e9dae51..db769cdcfaf9 100644
> > --- a/libsepol/src/services.c
> > +++ b/libsepol/src/services.c
> > @@ -394,7 +394,7 @@ static int constraint_expr_eval_reason(context_struct_t *scontext,
> > role_datum_t *r1, *r2;
> > mls_level_t *l1, *l2;
> > constraint_expr_t *e;
> > - int s[CEXPR_MAXDEPTH];
> > + int s[CEXPR_MAXDEPTH] = {};
> > int sp = -1;
> > char tmp_buf[128];
> >
> > --
> > 2.36.1
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-06 20:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-29 7:20 [PATCH userspace 1/1] libsepol: initialize s in constraint_expr_eval_reason Nicolas Iooss
2022-06-29 20:06 ` James Carter
2022-07-01 9:21 ` Christian Göttsche
2022-07-06 20:11 ` James Carter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.