| Message ID | 20190901180636.31586-4-nicolas.iooss@m4x.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Fix issues found by static analyzers | expand |
On 9/1/19 2:06 PM, Nicolas Iooss wrote: > When strs_stack_init(&stack) fails to allocate memory and stack is still > NULL, it should not be dereferenced with strs_stack_pop(stack). > > This issue has been found using Infer static analyzer. > > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> > --- > libsepol/src/kernel_to_cil.c | 8 +++++--- > libsepol/src/kernel_to_conf.c | 8 +++++--- > 2 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c > index 320af37b2bc8..9fcc254707ba 100644 > --- a/libsepol/src/kernel_to_cil.c > +++ b/libsepol/src/kernel_to_cil.c > @@ -108,10 +108,12 @@ static char *cond_expr_to_str(struct policydb *pdb, struct cond_expr *expr) > return str; > > exit: > - while ((new_val = strs_stack_pop(stack)) != NULL) { > - free(new_val); > + if (stack) { > + while ((new_val = strs_stack_pop(stack)) != NULL) { > + free(new_val); > + } > + strs_stack_destroy(&stack); > } > - strs_stack_destroy(&stack); > > return NULL; > } constraint_expr_to_str() has the same problem. > diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c > index 930bafabdd4b..2c8da49a11ab 100644 > --- a/libsepol/src/kernel_to_conf.c > +++ b/libsepol/src/kernel_to_conf.c > @@ -106,10 +106,12 @@ static char *cond_expr_to_str(struct policydb *pdb, struct cond_expr *expr) > return str; > > exit: > - while ((new_val = strs_stack_pop(stack)) != NULL) { > - free(new_val); > + if (stack != NULL) { > + while ((new_val = strs_stack_pop(stack)) != NULL) { > + free(new_val); > + } > + strs_stack_destroy(&stack); > } > - strs_stack_destroy(&stack); > > return NULL; > } > Same comment here. constraint_expr_to_str() also needs to be fixed.
On 9/10/19 2:52 PM, jwcart2 wrote: > On 9/1/19 2:06 PM, Nicolas Iooss wrote: >> When strs_stack_init(&stack) fails to allocate memory and stack is still >> NULL, it should not be dereferenced with strs_stack_pop(stack). >> >> This issue has been found using Infer static analyzer. >> >> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> >> --- >> libsepol/src/kernel_to_cil.c | 8 +++++--- >> libsepol/src/kernel_to_conf.c | 8 +++++--- >> 2 files changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c >> index 320af37b2bc8..9fcc254707ba 100644 >> --- a/libsepol/src/kernel_to_cil.c >> +++ b/libsepol/src/kernel_to_cil.c >> @@ -108,10 +108,12 @@ static char *cond_expr_to_str(struct policydb *pdb, >> struct cond_expr *expr) >> return str; >> exit: >> - while ((new_val = strs_stack_pop(stack)) != NULL) { >> - free(new_val); >> + if (stack) { >> + while ((new_val = strs_stack_pop(stack)) != NULL) { >> + free(new_val); >> + } >> + strs_stack_destroy(&stack); >> } >> - strs_stack_destroy(&stack); >> return NULL; >> } > > constraint_expr_to_str() has the same problem. > >> diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c >> index 930bafabdd4b..2c8da49a11ab 100644 >> --- a/libsepol/src/kernel_to_conf.c >> +++ b/libsepol/src/kernel_to_conf.c >> @@ -106,10 +106,12 @@ static char *cond_expr_to_str(struct policydb *pdb, >> struct cond_expr *expr) >> return str; >> exit: >> - while ((new_val = strs_stack_pop(stack)) != NULL) { >> - free(new_val); >> + if (stack != NULL) { I forgot to mention that you should just use "if (stack) {" like above so the syntax is consistent. Thanks, Jim >> + while ((new_val = strs_stack_pop(stack)) != NULL) { >> + free(new_val); >> + } >> + strs_stack_destroy(&stack); >> } >> - strs_stack_destroy(&stack); >> return NULL; >> } >> > > Same comment here. constraint_expr_to_str() also needs to be fixed. >
diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c index 320af37b2bc8..9fcc254707ba 100644 --- a/libsepol/src/kernel_to_cil.c +++ b/libsepol/src/kernel_to_cil.c @@ -108,10 +108,12 @@ static char *cond_expr_to_str(struct policydb *pdb, struct cond_expr *expr) return str; exit: - while ((new_val = strs_stack_pop(stack)) != NULL) { - free(new_val); + if (stack) { + while ((new_val = strs_stack_pop(stack)) != NULL) { + free(new_val); + } + strs_stack_destroy(&stack); } - strs_stack_destroy(&stack); return NULL; } diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c index 930bafabdd4b..2c8da49a11ab 100644 --- a/libsepol/src/kernel_to_conf.c +++ b/libsepol/src/kernel_to_conf.c @@ -106,10 +106,12 @@ static char *cond_expr_to_str(struct policydb *pdb, struct cond_expr *expr) return str; exit: - while ((new_val = strs_stack_pop(stack)) != NULL) { - free(new_val); + if (stack != NULL) { + while ((new_val = strs_stack_pop(stack)) != NULL) { + free(new_val); + } + strs_stack_destroy(&stack); } - strs_stack_destroy(&stack); return NULL; }
When strs_stack_init(&stack) fails to allocate memory and stack is still NULL, it should not be dereferenced with strs_stack_pop(stack). This issue has been found using Infer static analyzer. Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> --- libsepol/src/kernel_to_cil.c | 8 +++++--- libsepol/src/kernel_to_conf.c | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-)