| Message ID | 20230706135221.43544-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0e2a78d5b2a5 |
| Headers | show |
| Series | libsepol: free initial sid names | expand |
On Thu, Jul 6, 2023 at 10:01 AM Christian Göttsche <cgzones@googlemail.com> wrote: > > Commit 55b75a2c ("libsepol: stop translating deprecated intial SIDs to > strings") dropped several names of obsolete initial sids ans replaced > them with NULL. This leads to their printable string being dynamically > allocated but not free'd. > Instead of keeping track of which name was allocated dynamically and > which not, allocate all on the heap, which simplifies the later cleanup. > > While on it also free the name in case of a strs_add_at_index() failure. > > Reported-by: oss-fuzz (issue 60271) > Fixes: 55b75a2c ("libsepol: stop translating deprecated intial SIDs to strings") > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> Acked-by: James Carter <jwcart2@gmail.com> > --- > libsepol/src/kernel_to_cil.c | 18 ++++++++---------- > libsepol/src/kernel_to_conf.c | 16 +++++++--------- > 2 files changed, 15 insertions(+), 19 deletions(-) > > diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c > index a3d8d139..8fcc385d 100644 > --- a/libsepol/src/kernel_to_cil.c > +++ b/libsepol/src/kernel_to_cil.c > @@ -569,18 +569,19 @@ static int write_sids_to_cil(FILE *out, const char *const *sid_to_str, > for (isid = isids; isid != NULL; isid = isid->next) { > i = isid->sid[0]; > if (i < num_sids && sid_to_str[i]) { > - sid = (char *)sid_to_str[i]; > + sid = strdup(sid_to_str[i]); > } else { > snprintf(unknown, 18, "%s%u", "UNKNOWN", i); > sid = strdup(unknown); > - if (!sid) { > - ERR(NULL, "Out of memory"); > - rc = -1; > - goto exit; > - } > + } > + if (!sid) { > + ERR(NULL, "Out of memory"); > + rc = -1; > + goto exit; > } > rc = strs_add_at_index(strs, sid, i); > if (rc != 0) { > + free(sid); > goto exit; > } > } > @@ -611,10 +612,7 @@ static int write_sids_to_cil(FILE *out, const char *const *sid_to_str, > sepol_printf(out, "))\n"); > > exit: > - for (i=num_sids; i<strs_num_items(strs); i++) { > - sid = strs_read_at_index(strs, i); > - free(sid); > - } > + strs_free_all(strs); > strs_destroy(&strs); > if (rc != 0) { > ERR(NULL, "Error writing sid rules to CIL"); > diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c > index 0710572d..b0ae16d9 100644 > --- a/libsepol/src/kernel_to_conf.c > +++ b/libsepol/src/kernel_to_conf.c > @@ -466,17 +466,18 @@ static int write_sids_to_conf(FILE *out, const char *const *sid_to_str, > for (isid = isids; isid != NULL; isid = isid->next) { > i = isid->sid[0]; > if (i < num_sids && sid_to_str[i]) { > - sid = (char *)sid_to_str[i]; > + sid = strdup(sid_to_str[i]); > } else { > snprintf(unknown, sizeof(unknown), "%s%u", "UNKNOWN", i); > sid = strdup(unknown); > - if (!sid) { > - rc = -1; > - goto exit; > - } > + } > + if (!sid) { > + rc = -1; > + goto exit; > } > rc = strs_add_at_index(strs, sid, i); > if (rc != 0) { > + free(sid); > goto exit; > } > } > @@ -490,10 +491,7 @@ static int write_sids_to_conf(FILE *out, const char *const *sid_to_str, > } > > exit: > - for (i=num_sids; i<strs_num_items(strs); i++) { > - sid = strs_read_at_index(strs, i); > - free(sid); > - } > + strs_free_all(strs); > strs_destroy(&strs); > if (rc != 0) { > ERR(NULL, "Error writing sid rules to policy.conf"); > -- > 2.40.1 >
On Tue, Jul 11, 2023 at 2:45 PM James Carter <jwcart2@gmail.com> wrote: > > On Thu, Jul 6, 2023 at 10:01 AM Christian Göttsche > <cgzones@googlemail.com> wrote: > > > > Commit 55b75a2c ("libsepol: stop translating deprecated intial SIDs to > > strings") dropped several names of obsolete initial sids ans replaced > > them with NULL. This leads to their printable string being dynamically > > allocated but not free'd. > > Instead of keeping track of which name was allocated dynamically and > > which not, allocate all on the heap, which simplifies the later cleanup. > > > > While on it also free the name in case of a strs_add_at_index() failure. > > > > Reported-by: oss-fuzz (issue 60271) > > Fixes: 55b75a2c ("libsepol: stop translating deprecated intial SIDs to strings") > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > Acked-by: James Carter <jwcart2@gmail.com> > Merged. Thanks, Jim > > --- > > libsepol/src/kernel_to_cil.c | 18 ++++++++---------- > > libsepol/src/kernel_to_conf.c | 16 +++++++--------- > > 2 files changed, 15 insertions(+), 19 deletions(-) > > > > diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c > > index a3d8d139..8fcc385d 100644 > > --- a/libsepol/src/kernel_to_cil.c > > +++ b/libsepol/src/kernel_to_cil.c > > @@ -569,18 +569,19 @@ static int write_sids_to_cil(FILE *out, const char *const *sid_to_str, > > for (isid = isids; isid != NULL; isid = isid->next) { > > i = isid->sid[0]; > > if (i < num_sids && sid_to_str[i]) { > > - sid = (char *)sid_to_str[i]; > > + sid = strdup(sid_to_str[i]); > > } else { > > snprintf(unknown, 18, "%s%u", "UNKNOWN", i); > > sid = strdup(unknown); > > - if (!sid) { > > - ERR(NULL, "Out of memory"); > > - rc = -1; > > - goto exit; > > - } > > + } > > + if (!sid) { > > + ERR(NULL, "Out of memory"); > > + rc = -1; > > + goto exit; > > } > > rc = strs_add_at_index(strs, sid, i); > > if (rc != 0) { > > + free(sid); > > goto exit; > > } > > } > > @@ -611,10 +612,7 @@ static int write_sids_to_cil(FILE *out, const char *const *sid_to_str, > > sepol_printf(out, "))\n"); > > > > exit: > > - for (i=num_sids; i<strs_num_items(strs); i++) { > > - sid = strs_read_at_index(strs, i); > > - free(sid); > > - } > > + strs_free_all(strs); > > strs_destroy(&strs); > > if (rc != 0) { > > ERR(NULL, "Error writing sid rules to CIL"); > > diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c > > index 0710572d..b0ae16d9 100644 > > --- a/libsepol/src/kernel_to_conf.c > > +++ b/libsepol/src/kernel_to_conf.c > > @@ -466,17 +466,18 @@ static int write_sids_to_conf(FILE *out, const char *const *sid_to_str, > > for (isid = isids; isid != NULL; isid = isid->next) { > > i = isid->sid[0]; > > if (i < num_sids && sid_to_str[i]) { > > - sid = (char *)sid_to_str[i]; > > + sid = strdup(sid_to_str[i]); > > } else { > > snprintf(unknown, sizeof(unknown), "%s%u", "UNKNOWN", i); > > sid = strdup(unknown); > > - if (!sid) { > > - rc = -1; > > - goto exit; > > - } > > + } > > + if (!sid) { > > + rc = -1; > > + goto exit; > > } > > rc = strs_add_at_index(strs, sid, i); > > if (rc != 0) { > > + free(sid); > > goto exit; > > } > > } > > @@ -490,10 +491,7 @@ static int write_sids_to_conf(FILE *out, const char *const *sid_to_str, > > } > > > > exit: > > - for (i=num_sids; i<strs_num_items(strs); i++) { > > - sid = strs_read_at_index(strs, i); > > - free(sid); > > - } > > + strs_free_all(strs); > > strs_destroy(&strs); > > if (rc != 0) { > > ERR(NULL, "Error writing sid rules to policy.conf"); > > -- > > 2.40.1 > >
diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c index a3d8d139..8fcc385d 100644 --- a/libsepol/src/kernel_to_cil.c +++ b/libsepol/src/kernel_to_cil.c @@ -569,18 +569,19 @@ static int write_sids_to_cil(FILE *out, const char *const *sid_to_str, for (isid = isids; isid != NULL; isid = isid->next) { i = isid->sid[0]; if (i < num_sids && sid_to_str[i]) { - sid = (char *)sid_to_str[i]; + sid = strdup(sid_to_str[i]); } else { snprintf(unknown, 18, "%s%u", "UNKNOWN", i); sid = strdup(unknown); - if (!sid) { - ERR(NULL, "Out of memory"); - rc = -1; - goto exit; - } + } + if (!sid) { + ERR(NULL, "Out of memory"); + rc = -1; + goto exit; } rc = strs_add_at_index(strs, sid, i); if (rc != 0) { + free(sid); goto exit; } } @@ -611,10 +612,7 @@ static int write_sids_to_cil(FILE *out, const char *const *sid_to_str, sepol_printf(out, "))\n"); exit: - for (i=num_sids; i<strs_num_items(strs); i++) { - sid = strs_read_at_index(strs, i); - free(sid); - } + strs_free_all(strs); strs_destroy(&strs); if (rc != 0) { ERR(NULL, "Error writing sid rules to CIL"); diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c index 0710572d..b0ae16d9 100644 --- a/libsepol/src/kernel_to_conf.c +++ b/libsepol/src/kernel_to_conf.c @@ -466,17 +466,18 @@ static int write_sids_to_conf(FILE *out, const char *const *sid_to_str, for (isid = isids; isid != NULL; isid = isid->next) { i = isid->sid[0]; if (i < num_sids && sid_to_str[i]) { - sid = (char *)sid_to_str[i]; + sid = strdup(sid_to_str[i]); } else { snprintf(unknown, sizeof(unknown), "%s%u", "UNKNOWN", i); sid = strdup(unknown); - if (!sid) { - rc = -1; - goto exit; - } + } + if (!sid) { + rc = -1; + goto exit; } rc = strs_add_at_index(strs, sid, i); if (rc != 0) { + free(sid); goto exit; } } @@ -490,10 +491,7 @@ static int write_sids_to_conf(FILE *out, const char *const *sid_to_str, } exit: - for (i=num_sids; i<strs_num_items(strs); i++) { - sid = strs_read_at_index(strs, i); - free(sid); - } + strs_free_all(strs); strs_destroy(&strs); if (rc != 0) { ERR(NULL, "Error writing sid rules to policy.conf");
Commit 55b75a2c ("libsepol: stop translating deprecated intial SIDs to strings") dropped several names of obsolete initial sids ans replaced them with NULL. This leads to their printable string being dynamically allocated but not free'd. Instead of keeping track of which name was allocated dynamically and which not, allocate all on the heap, which simplifies the later cleanup. While on it also free the name in case of a strs_add_at_index() failure. Reported-by: oss-fuzz (issue 60271) Fixes: 55b75a2c ("libsepol: stop translating deprecated intial SIDs to strings") Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- libsepol/src/kernel_to_cil.c | 18 ++++++++---------- libsepol/src/kernel_to_conf.c | 16 +++++++--------- 2 files changed, 15 insertions(+), 19 deletions(-)