diff mbox series

[14/15] checkpolicy: avoid assigning garbage values

Message ID 20240122135507.63506-14-cgzones@googlemail.com (mailing list archive)
State Accepted
Commit 22f7bb8c2ace
Delegated to: Petr Lautrbach
Headers show
Series [01/15] checkpolicy: add libfuzz based fuzzer | expand

Commit Message

Christian Göttsche Jan. 22, 2024, 1:55 p.m. UTC
Only assign the computed value on success, since it is not set by
declare_symbol() on failure.

Reported by GCC:

    module_compiler.c: In function 'create_role':
    module_compiler.c:287:24: warning: use of uninitialized value 'value' [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
      287 |         datum->s.value = value;
          |         ~~~~~~~~~~~~~~~^~~~~~~

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/module_compiler.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

James Carter Feb. 13, 2024, 8:38 p.m. UTC | #1
On Mon, Jan 22, 2024 at 9:02 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Only assign the computed value on success, since it is not set by
> declare_symbol() on failure.
>
> Reported by GCC:
>
>     module_compiler.c: In function 'create_role':
>     module_compiler.c:287:24: warning: use of uninitialized value 'value' [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
>       287 |         datum->s.value = value;
>           |         ~~~~~~~~~~~~~~~^~~~~~~
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Acked-by: James Carter <jwcart2@gmail.com>

> ---
>  checkpolicy/module_compiler.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
> index 464897cc..6ff91b8f 100644
> --- a/checkpolicy/module_compiler.c
> +++ b/checkpolicy/module_compiler.c
> @@ -284,9 +284,8 @@ static int create_role(uint32_t scope, unsigned char isattr, role_datum_t **role
>                 ret = require_symbol(SYM_ROLES, id, datum, &value, &value);
>         }
>
> -       datum->s.value = value;
> -
>         if (ret == 0) {
> +               datum->s.value = value;
>                 *role = datum;
>                 *key = strdup(id);
>                 if (*key == NULL) {
> @@ -303,6 +302,7 @@ static int create_role(uint32_t scope, unsigned char isattr, role_datum_t **role
>                         free(datum);
>                         return -1;
>                 }
> +               datum->s.value = value;
>                 *role = datum;
>                 *key = id;
>         } else {
> @@ -529,9 +529,8 @@ static int create_user(uint32_t scope, user_datum_t **user, char **key)
>                 ret = require_symbol(SYM_USERS, id, datum, &value, &value);
>         }
>
> -       datum->s.value = value;
> -
>         if (ret == 0) {
> +               datum->s.value = value;
>                 *user = datum;
>                 *key = strdup(id);
>                 if (*key == NULL) {
> @@ -539,6 +538,7 @@ static int create_user(uint32_t scope, user_datum_t **user, char **key)
>                         return -1;
>                 }
>         } else if (ret == 1) {
> +               datum->s.value = value;
>                 *user = datum;
>                 *key = id;
>         } else {
> --
> 2.43.0
>
>
James Carter March 4, 2024, 7:20 p.m. UTC | #2
On Tue, Feb 13, 2024 at 3:38 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Mon, Jan 22, 2024 at 9:02 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Only assign the computed value on success, since it is not set by
> > declare_symbol() on failure.
> >
> > Reported by GCC:
> >
> >     module_compiler.c: In function 'create_role':
> >     module_compiler.c:287:24: warning: use of uninitialized value 'value' [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
> >       287 |         datum->s.value = value;
> >           |         ~~~~~~~~~~~~~~~^~~~~~~
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> Acked-by: James Carter <jwcart2@gmail.com>
>
Merged.
Thanks,
Jim

> > ---
> >  checkpolicy/module_compiler.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
> > index 464897cc..6ff91b8f 100644
> > --- a/checkpolicy/module_compiler.c
> > +++ b/checkpolicy/module_compiler.c
> > @@ -284,9 +284,8 @@ static int create_role(uint32_t scope, unsigned char isattr, role_datum_t **role
> >                 ret = require_symbol(SYM_ROLES, id, datum, &value, &value);
> >         }
> >
> > -       datum->s.value = value;
> > -
> >         if (ret == 0) {
> > +               datum->s.value = value;
> >                 *role = datum;
> >                 *key = strdup(id);
> >                 if (*key == NULL) {
> > @@ -303,6 +302,7 @@ static int create_role(uint32_t scope, unsigned char isattr, role_datum_t **role
> >                         free(datum);
> >                         return -1;
> >                 }
> > +               datum->s.value = value;
> >                 *role = datum;
> >                 *key = id;
> >         } else {
> > @@ -529,9 +529,8 @@ static int create_user(uint32_t scope, user_datum_t **user, char **key)
> >                 ret = require_symbol(SYM_USERS, id, datum, &value, &value);
> >         }
> >
> > -       datum->s.value = value;
> > -
> >         if (ret == 0) {
> > +               datum->s.value = value;
> >                 *user = datum;
> >                 *key = strdup(id);
> >                 if (*key == NULL) {
> > @@ -539,6 +538,7 @@ static int create_user(uint32_t scope, user_datum_t **user, char **key)
> >                         return -1;
> >                 }
> >         } else if (ret == 1) {
> > +               datum->s.value = value;
> >                 *user = datum;
> >                 *key = id;
> >         } else {
> > --
> > 2.43.0
> >
> >
diff mbox series

Patch

diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
index 464897cc..6ff91b8f 100644
--- a/checkpolicy/module_compiler.c
+++ b/checkpolicy/module_compiler.c
@@ -284,9 +284,8 @@  static int create_role(uint32_t scope, unsigned char isattr, role_datum_t **role
 		ret = require_symbol(SYM_ROLES, id, datum, &value, &value);
 	}
 
-	datum->s.value = value;
-
 	if (ret == 0) {
+		datum->s.value = value;
 		*role = datum;
 		*key = strdup(id);
 		if (*key == NULL) {
@@ -303,6 +302,7 @@  static int create_role(uint32_t scope, unsigned char isattr, role_datum_t **role
 			free(datum);
 			return -1;
 		}
+		datum->s.value = value;
 		*role = datum;
 		*key = id;
 	} else {
@@ -529,9 +529,8 @@  static int create_user(uint32_t scope, user_datum_t **user, char **key)
 		ret = require_symbol(SYM_USERS, id, datum, &value, &value);
 	}
 
-	datum->s.value = value;
-
 	if (ret == 0) {
+		datum->s.value = value;
 		*user = datum;
 		*key = strdup(id);
 		if (*key == NULL) {
@@ -539,6 +538,7 @@  static int create_user(uint32_t scope, user_datum_t **user, char **key)
 			return -1;
 		}
 	} else if (ret == 1) {
+		datum->s.value = value;
 		*user = datum;
 		*key = id;
 	} else {