* [PATCH 1/3] libselinux: fix potential NULL reference and memory leak in audit2why
@ 2022-12-05 9:36 Jie Lu
2022-12-05 9:36 ` [PATCH 2/3] libsepol: fix potential memory leak in common_copy_callback Jie Lu
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jie Lu @ 2022-12-05 9:36 UTC (permalink / raw)
To: selinux
In audit2why.c add return check for memory allocation. And free every element
in the boollist when function fails.
Signed-off-by: Jie Lu <lujie54@huawei.com>
---
libselinux/src/audit2why.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/libselinux/src/audit2why.c b/libselinux/src/audit2why.c
index ba1a66eb..742b4ff5 100644
--- a/libselinux/src/audit2why.c
+++ b/libselinux/src/audit2why.c
@@ -55,7 +55,16 @@ static int load_booleans(const sepol_bool_t * boolean,
void *arg __attribute__ ((__unused__)))
{
boollist[boolcnt] = malloc(sizeof(struct boolean_t));
+ if (!boollist[boolcnt]) {
+ PyErr_SetString( PyExc_MemoryError, "Out of memory\n");
+ return -1;
+ }
boollist[boolcnt]->name = strdup(sepol_bool_get_name(boolean));
+ if (!boollist[boolcnt]->name) {
+ PyErr_SetString( PyExc_MemoryError, "Out of memory\n");
+ free(boollist[boolcnt]);
+ return -1;
+ }
boollist[boolcnt]->active = sepol_bool_get_value(boolean);
boolcnt++;
return 0;
@@ -149,6 +158,11 @@ static int check_booleans(struct boolean_t **bools)
if (fcnt > 0) {
*bools = calloc(sizeof(struct boolean_t), fcnt + 1);
+ if (!*bools) {
+ PyErr_SetString( PyExc_MemoryError, "Out of memory\n");
+ free(foundlist);
+ return 0;
+ }
struct boolean_t *b = *bools;
for (i = 0; i < fcnt; i++) {
int ctr = foundlist[i];
@@ -278,14 +292,22 @@ static int __policy_init(const char *init_path)
return 0;
err:
- if (boollist)
- free(boollist);
+ if (boollist) {
+ for (i = 0; i < boolcnt; i++) {
+ free(boollist[i]->name);
+ free(boollist[i]);
+ }
+ free(boollist);
+ boollist = NULL;
+ boolcnt = 0;
+ }
if (avc){
if (avc->handle)
sepol_handle_destroy(avc->handle);
if (avc->policydb)
sepol_policydb_free(avc->policydb);
free(avc);
+ avc = NULL;
}
if (pf)
sepol_policy_file_free(pf);
--
2.27.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] libsepol: fix potential memory leak in common_copy_callback
2022-12-05 9:36 [PATCH 1/3] libselinux: fix potential NULL reference and memory leak in audit2why Jie Lu
@ 2022-12-05 9:36 ` Jie Lu
2022-12-16 16:06 ` James Carter
2022-12-05 9:36 ` [PATCH 3/3] policycoreutils: fix potential NULL reference in load_checks Jie Lu
2022-12-16 16:03 ` [PATCH 1/3] libselinux: fix potential NULL reference and memory leak in audit2why James Carter
2 siblings, 1 reply; 7+ messages in thread
From: Jie Lu @ 2022-12-05 9:36 UTC (permalink / raw)
To: selinux
In common_copy_callback(), destroy new_common->permissions when the function fails.
Signed-off-by: Jie Lu <lujie54@huawei.com>
---
libsepol/src/expand.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index 8d19850e..b44cfd4f 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -297,6 +297,7 @@ static int common_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
(hashtab_datum_t) new_common);
if (ret) {
ERR(state->handle, "hashtab overflow");
+ symtab_destroy(&new_common->permissions);
free(new_common);
free(new_id);
return -1;
--
2.27.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] policycoreutils: fix potential NULL reference in load_checks
2022-12-05 9:36 [PATCH 1/3] libselinux: fix potential NULL reference and memory leak in audit2why Jie Lu
2022-12-05 9:36 ` [PATCH 2/3] libsepol: fix potential memory leak in common_copy_callback Jie Lu
@ 2022-12-05 9:36 ` Jie Lu
2022-12-16 16:07 ` James Carter
2022-12-16 16:03 ` [PATCH 1/3] libselinux: fix potential NULL reference and memory leak in audit2why James Carter
2 siblings, 1 reply; 7+ messages in thread
From: Jie Lu @ 2022-12-05 9:36 UTC (permalink / raw)
To: selinux
In load_checks(), add return check for malloc() to avoid NULL reference.
Signed-off-by: Jie Lu <lujie54@huawei.com>
---
policycoreutils/sestatus/sestatus.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/policycoreutils/sestatus/sestatus.c b/policycoreutils/sestatus/sestatus.c
index 7dcc9944..6c95828e 100644
--- a/policycoreutils/sestatus/sestatus.c
+++ b/policycoreutils/sestatus/sestatus.c
@@ -140,6 +140,8 @@ static void load_checks(char *pc[], int *npc, char *fc[], int *nfc)
pc[*npc] =
(char *)malloc((buf_len) *
sizeof(char));
+ if (!pc[*npc])
+ break;
memcpy(pc[*npc], bufp, buf_len);
(*npc)++;
bufp = NULL;
@@ -150,6 +152,8 @@ static void load_checks(char *pc[], int *npc, char *fc[], int *nfc)
fc[*nfc] =
(char *)malloc((buf_len) *
sizeof(char));
+ if (!fc[*nfc])
+ break;
memcpy(fc[*nfc], bufp, buf_len);
(*nfc)++;
bufp = NULL;
--
2.27.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] libselinux: fix potential NULL reference and memory leak in audit2why
2022-12-05 9:36 [PATCH 1/3] libselinux: fix potential NULL reference and memory leak in audit2why Jie Lu
2022-12-05 9:36 ` [PATCH 2/3] libsepol: fix potential memory leak in common_copy_callback Jie Lu
2022-12-05 9:36 ` [PATCH 3/3] policycoreutils: fix potential NULL reference in load_checks Jie Lu
@ 2022-12-16 16:03 ` James Carter
2 siblings, 0 replies; 7+ messages in thread
From: James Carter @ 2022-12-16 16:03 UTC (permalink / raw)
To: Jie Lu; +Cc: selinux
On Mon, Dec 5, 2022 at 7:13 AM Jie Lu <lujie54@huawei.com> wrote:
>
> In audit2why.c add return check for memory allocation. And free every element
> in the boollist when function fails.
>
> Signed-off-by: Jie Lu <lujie54@huawei.com>
> ---
> libselinux/src/audit2why.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/libselinux/src/audit2why.c b/libselinux/src/audit2why.c
> index ba1a66eb..742b4ff5 100644
> --- a/libselinux/src/audit2why.c
> +++ b/libselinux/src/audit2why.c
> @@ -55,7 +55,16 @@ static int load_booleans(const sepol_bool_t * boolean,
> void *arg __attribute__ ((__unused__)))
> {
> boollist[boolcnt] = malloc(sizeof(struct boolean_t));
> + if (!boollist[boolcnt]) {
> + PyErr_SetString( PyExc_MemoryError, "Out of memory\n");
> + return -1;
> + }
> boollist[boolcnt]->name = strdup(sepol_bool_get_name(boolean));
> + if (!boollist[boolcnt]->name) {
> + PyErr_SetString( PyExc_MemoryError, "Out of memory\n");
> + free(boollist[boolcnt]);
> + return -1;
> + }
> boollist[boolcnt]->active = sepol_bool_get_value(boolean);
> boolcnt++;
> return 0;
> @@ -149,6 +158,11 @@ static int check_booleans(struct boolean_t **bools)
>
> if (fcnt > 0) {
> *bools = calloc(sizeof(struct boolean_t), fcnt + 1);
> + if (!*bools) {
> + PyErr_SetString( PyExc_MemoryError, "Out of memory\n");
> + free(foundlist);
> + return 0;
> + }
> struct boolean_t *b = *bools;
> for (i = 0; i < fcnt; i++) {
> int ctr = foundlist[i];
> @@ -278,14 +292,22 @@ static int __policy_init(const char *init_path)
> return 0;
>
> err:
> - if (boollist)
> - free(boollist);
> + if (boollist) {
> + for (i = 0; i < boolcnt; i++) {
> + free(boollist[i]->name);
> + free(boollist[i]);
> + }
> + free(boollist);
> + boollist = NULL;
> + boolcnt = 0;
> + }
i is not declared and it is indented with spaces rather than tabs.
Thanks,
Jim
> if (avc){
> if (avc->handle)
> sepol_handle_destroy(avc->handle);
> if (avc->policydb)
> sepol_policydb_free(avc->policydb);
> free(avc);
> + avc = NULL;
> }
> if (pf)
> sepol_policy_file_free(pf);
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] libsepol: fix potential memory leak in common_copy_callback
2022-12-05 9:36 ` [PATCH 2/3] libsepol: fix potential memory leak in common_copy_callback Jie Lu
@ 2022-12-16 16:06 ` James Carter
0 siblings, 0 replies; 7+ messages in thread
From: James Carter @ 2022-12-16 16:06 UTC (permalink / raw)
To: Jie Lu; +Cc: selinux
On Mon, Dec 5, 2022 at 7:13 AM Jie Lu <lujie54@huawei.com> wrote:
>
> In common_copy_callback(), destroy new_common->permissions when the function fails.
>
> Signed-off-by: Jie Lu <lujie54@huawei.com>
> ---
> libsepol/src/expand.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> index 8d19850e..b44cfd4f 100644
> --- a/libsepol/src/expand.c
> +++ b/libsepol/src/expand.c
> @@ -297,6 +297,7 @@ static int common_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
> (hashtab_datum_t) new_common);
> if (ret) {
> ERR(state->handle, "hashtab overflow");
> + symtab_destroy(&new_common->permissions);
> free(new_common);
> free(new_id);
> return -1;
The call to hashtab_map() below this also needs similar cleanup if
there is an error. In fact, this whole function really needs common
cleanup code with a goto that code upon an error.
Thanks,
Jim
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] policycoreutils: fix potential NULL reference in load_checks
2022-12-05 9:36 ` [PATCH 3/3] policycoreutils: fix potential NULL reference in load_checks Jie Lu
@ 2022-12-16 16:07 ` James Carter
2023-01-11 15:51 ` James Carter
0 siblings, 1 reply; 7+ messages in thread
From: James Carter @ 2022-12-16 16:07 UTC (permalink / raw)
To: Jie Lu; +Cc: selinux
On Mon, Dec 5, 2022 at 7:13 AM Jie Lu <lujie54@huawei.com> wrote:
>
> In load_checks(), add return check for malloc() to avoid NULL reference.
>
> Signed-off-by: Jie Lu <lujie54@huawei.com>
Acked-by: James Carter <jwcart2@gmail.com>
> ---
> policycoreutils/sestatus/sestatus.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/policycoreutils/sestatus/sestatus.c b/policycoreutils/sestatus/sestatus.c
> index 7dcc9944..6c95828e 100644
> --- a/policycoreutils/sestatus/sestatus.c
> +++ b/policycoreutils/sestatus/sestatus.c
> @@ -140,6 +140,8 @@ static void load_checks(char *pc[], int *npc, char *fc[], int *nfc)
> pc[*npc] =
> (char *)malloc((buf_len) *
> sizeof(char));
> + if (!pc[*npc])
> + break;
> memcpy(pc[*npc], bufp, buf_len);
> (*npc)++;
> bufp = NULL;
> @@ -150,6 +152,8 @@ static void load_checks(char *pc[], int *npc, char *fc[], int *nfc)
> fc[*nfc] =
> (char *)malloc((buf_len) *
> sizeof(char));
> + if (!fc[*nfc])
> + break;
> memcpy(fc[*nfc], bufp, buf_len);
> (*nfc)++;
> bufp = NULL;
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] policycoreutils: fix potential NULL reference in load_checks
2022-12-16 16:07 ` James Carter
@ 2023-01-11 15:51 ` James Carter
0 siblings, 0 replies; 7+ messages in thread
From: James Carter @ 2023-01-11 15:51 UTC (permalink / raw)
To: Jie Lu; +Cc: selinux
On Fri, Dec 16, 2022 at 11:07 AM James Carter <jwcart2@gmail.com> wrote:
>
> On Mon, Dec 5, 2022 at 7:13 AM Jie Lu <lujie54@huawei.com> wrote:
> >
> > In load_checks(), add return check for malloc() to avoid NULL reference.
> >
> > Signed-off-by: Jie Lu <lujie54@huawei.com>
>
> Acked-by: James Carter <jwcart2@gmail.com>
This patch, but not the other two, has been merged.
Thanks,
Jim
>
> > ---
> > policycoreutils/sestatus/sestatus.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/policycoreutils/sestatus/sestatus.c b/policycoreutils/sestatus/sestatus.c
> > index 7dcc9944..6c95828e 100644
> > --- a/policycoreutils/sestatus/sestatus.c
> > +++ b/policycoreutils/sestatus/sestatus.c
> > @@ -140,6 +140,8 @@ static void load_checks(char *pc[], int *npc, char *fc[], int *nfc)
> > pc[*npc] =
> > (char *)malloc((buf_len) *
> > sizeof(char));
> > + if (!pc[*npc])
> > + break;
> > memcpy(pc[*npc], bufp, buf_len);
> > (*npc)++;
> > bufp = NULL;
> > @@ -150,6 +152,8 @@ static void load_checks(char *pc[], int *npc, char *fc[], int *nfc)
> > fc[*nfc] =
> > (char *)malloc((buf_len) *
> > sizeof(char));
> > + if (!fc[*nfc])
> > + break;
> > memcpy(fc[*nfc], bufp, buf_len);
> > (*nfc)++;
> > bufp = NULL;
> > --
> > 2.27.0
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-01-11 15:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-05 9:36 [PATCH 1/3] libselinux: fix potential NULL reference and memory leak in audit2why Jie Lu
2022-12-05 9:36 ` [PATCH 2/3] libsepol: fix potential memory leak in common_copy_callback Jie Lu
2022-12-16 16:06 ` James Carter
2022-12-05 9:36 ` [PATCH 3/3] policycoreutils: fix potential NULL reference in load_checks Jie Lu
2022-12-16 16:07 ` James Carter
2023-01-11 15:51 ` James Carter
2022-12-16 16:03 ` [PATCH 1/3] libselinux: fix potential NULL reference and memory leak in audit2why 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.