[PATCH] libsepol: Prevent freeing unitialized value in ibendport handling

[James Carter]

Nicolas Iooss reports:
In sepol_ibendport_key_create(), if sepol_ibendport_alloc_ibdev_name()
fails to allocate tmp_key->ibdev_name, sepol_ibendport_key_free() is
called to free the memory associated with tmp_key, which results in
free() being called on uninitialized tmp_key->ibdev_name.

This issue is reported by clang's static analyzer with the following
message:

ibendport_record.c:115:2: warning: 1st function call argument is an
uninitialized value
free(key->ibdev_name);
^~~~~~~~~~~~~~~~~~~~~

Signed-off-by: James Carter <***@tycho.nsa.gov>
---
libsepol/src/ibendport_record.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/libsepol/src/ibendport_record.c b/libsepol/src/ibendport_record.c
index 912aeb53..bc56f090 100644
--- a/libsepol/src/ibendport_record.c
+++ b/libsepol/src/ibendport_record.c
@@ -32,14 +32,11 @@ struct sepol_ibendport_key {
int sepol_ibendport_alloc_ibdev_name(sepol_handle_t *handle,
char **ibdev_name)
{
- char *tmp_ibdev_name = NULL;
-
- tmp_ibdev_name = calloc(1, IB_DEVICE_NAME_MAX);
+ *ibdev_name = calloc(1, IB_DEVICE_NAME_MAX);

- if (!tmp_ibdev_name)
+ if (!*ibdev_name)
goto omem;

- *ibdev_name = tmp_ibdev_name;
return STATUS_SUCCESS;

omem:

[Nicolas Iooss]

Post by James Carter
In sepol_ibendport_key_create(), if sepol_ibendport_alloc_ibdev_name()
fails to allocate tmp_key->ibdev_name, sepol_ibendport_key_free() is
called to free the memory associated with tmp_key, which results in
free() being called on uninitialized tmp_key->ibdev_name.
This issue is reported by clang's static analyzer with the following
ibendport_record.c:115:2: warning: 1st function call argument is an
uninitialized value
free(key->ibdev_name);
^~~~~~~~~~~~~~~~~~~~~
---
libsepol/src/ibendport_record.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/libsepol/src/ibendport_record.c b/libsepol/src/ibendport_record.c
index 912aeb53..bc56f090 100644
--- a/libsepol/src/ibendport_record.c
+++ b/libsepol/src/ibendport_record.c
@@ -32,14 +32,11 @@ struct sepol_ibendport_key {
int sepol_ibendport_alloc_ibdev_name(sepol_handle_t *handle,
char **ibdev_name)
{
- char *tmp_ibdev_name = NULL;
-
- tmp_ibdev_name = calloc(1, IB_DEVICE_NAME_MAX);
+ *ibdev_name = calloc(1, IB_DEVICE_NAME_MAX);
- if (!tmp_ibdev_name)
+ if (!*ibdev_name)
goto omem;
- *ibdev_name = tmp_ibdev_name;
return STATUS_SUCCESS;
--
2.13.6

This patch looks good to me and I confirm clang's static analyzer no
longer reports the issue I had.

Acked-by: Nicolas Iooss <***@m4x.org>

Thanks,
Nicolas

[jwcart2]

Post by Nicolas Iooss

Post by James Carter
In sepol_ibendport_key_create(), if sepol_ibendport_alloc_ibdev_name()
fails to allocate tmp_key->ibdev_name, sepol_ibendport_key_free() is
called to free the memory associated with tmp_key, which results in
free() being called on uninitialized tmp_key->ibdev_name.
This issue is reported by clang's static analyzer with the following
ibendport_record.c:115:2: warning: 1st function call argument is an
uninitialized value
free(key->ibdev_name);
^~~~~~~~~~~~~~~~~~~~~
---
libsepol/src/ibendport_record.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/libsepol/src/ibendport_record.c b/libsepol/src/ibendport_record.c
index 912aeb53..bc56f090 100644
--- a/libsepol/src/ibendport_record.c
+++ b/libsepol/src/ibendport_record.c
@@ -32,14 +32,11 @@ struct sepol_ibendport_key {
int sepol_ibendport_alloc_ibdev_name(sepol_handle_t *handle,
char **ibdev_name)
{
- char *tmp_ibdev_name = NULL;
-
- tmp_ibdev_name = calloc(1, IB_DEVICE_NAME_MAX);
+ *ibdev_name = calloc(1, IB_DEVICE_NAME_MAX);
- if (!tmp_ibdev_name)
+ if (!*ibdev_name)
goto omem;
- *ibdev_name = tmp_ibdev_name;
return STATUS_SUCCESS;
--
2.13.6

This patch looks good to me and I confirm clang's static analyzer no
longer reports the issue I had.
Thanks,
Nicolas

This has been applied.

Jim