From: Tom Rix <[email protected]>
clang static analysis reports an undefined return
security/selinux/ss/conditional.c:79:2: warning: Undefined or garbage value returned to caller [core.uninitialized.UndefReturn]
return s[0];
^~~~~~~~~~~
static int cond_evaluate_expr( ...
{
u32 i;
int s[COND_EXPR_MAXDEPTH];
for (i = 0; i < expr->len; i++)
...
return s[0];
When expr->len is 0, the loop which sets s[0] never runs.
So return -1 if the loop never runs.
Signed-off-by: Tom Rix <[email protected]>
---
security/selinux/ss/conditional.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
index 450bc02f4cd2..0cc7cdd58465 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -27,6 +27,9 @@ static int cond_evaluate_expr(struct policydb *p, struct cond_expr *expr)
int s[COND_EXPR_MAXDEPTH];
int sp = -1;
+ if (expr->len == 0)
+ return -1;
+
for (i = 0; i < expr->len; i++) {
struct cond_expr_node *node = &expr->nodes[i];
--
2.18.1
On Wed, Jun 17, 2020 at 8:40 AM <[email protected]> wrote:
>
> From: Tom Rix <[email protected]>
>
> clang static analysis reports an undefined return
>
> security/selinux/ss/conditional.c:79:2: warning: Undefined or garbage value returned to caller [core.uninitialized.UndefReturn]
> return s[0];
> ^~~~~~~~~~~
>
> static int cond_evaluate_expr( ...
> {
> u32 i;
> int s[COND_EXPR_MAXDEPTH];
>
> for (i = 0; i < expr->len; i++)
> ...
>
> return s[0];
>
> When expr->len is 0, the loop which sets s[0] never runs.
>
> So return -1 if the loop never runs.
>
> Signed-off-by: Tom Rix <[email protected]>
Acked-by: Stephen Smalley <[email protected]>
clang didn't complain about the similar pattern in
security/selinux/ss/services.c:constraint_expr_eval()?
On Wed, Jun 17, 2020 at 9:58 AM Stephen Smalley
<[email protected]> wrote:
> On Wed, Jun 17, 2020 at 8:40 AM <[email protected]> wrote:
> >
> > From: Tom Rix <[email protected]>
> >
> > clang static analysis reports an undefined return
> >
> > security/selinux/ss/conditional.c:79:2: warning: Undefined or garbage value returned to caller [core.uninitialized.UndefReturn]
> > return s[0];
> > ^~~~~~~~~~~
> >
> > static int cond_evaluate_expr( ...
> > {
> > u32 i;
> > int s[COND_EXPR_MAXDEPTH];
> >
> > for (i = 0; i < expr->len; i++)
> > ...
> >
> > return s[0];
> >
> > When expr->len is 0, the loop which sets s[0] never runs.
> >
> > So return -1 if the loop never runs.
> >
> > Signed-off-by: Tom Rix <[email protected]>
>
> Acked-by: Stephen Smalley <[email protected]>
>
> clang didn't complain about the similar pattern in
> security/selinux/ss/services.c:constraint_expr_eval()?
Related question: I appreciate the work you are doing Tom, can you
share how far along you are testing the SELinux code with clang? I
ask because it would be nice to roll all of these patches up into one
PR for Linus instead of sending multiple updates.
--
paul moore
http://www.paul-moore.com
On Wed, Jun 17, 2020 at 4:51 PM Paul Moore <[email protected]> wrote:
> On Wed, Jun 17, 2020 at 9:58 AM Stephen Smalley
> <[email protected]> wrote:
> > On Wed, Jun 17, 2020 at 8:40 AM <[email protected]> wrote:
> > >
> > > From: Tom Rix <[email protected]>
> > >
> > > clang static analysis reports an undefined return
> > >
> > > security/selinux/ss/conditional.c:79:2: warning: Undefined or garbage value returned to caller [core.uninitialized.UndefReturn]
> > > return s[0];
> > > ^~~~~~~~~~~
> > >
> > > static int cond_evaluate_expr( ...
> > > {
> > > u32 i;
> > > int s[COND_EXPR_MAXDEPTH];
> > >
> > > for (i = 0; i < expr->len; i++)
> > > ...
> > >
> > > return s[0];
> > >
> > > When expr->len is 0, the loop which sets s[0] never runs.
> > >
> > > So return -1 if the loop never runs.
> > >
> > > Signed-off-by: Tom Rix <[email protected]>
> >
> > Acked-by: Stephen Smalley <[email protected]>
> >
> > clang didn't complain about the similar pattern in
> > security/selinux/ss/services.c:constraint_expr_eval()?
>
> Related question: I appreciate the work you are doing Tom, can you
> share how far along you are testing the SELinux code with clang? I
> ask because it would be nice to roll all of these patches up into one
> PR for Linus instead of sending multiple updates.
Regardless, this patch looks good to me too so I've merged it into the
selinux/stable-5.8 branch with the others. Thank you.
It would still be nice to know if there are other clang failures you
are working on fixing or if this is it for awhile.
--
paul moore
http://www.paul-moore.com