[PATCH] libsepol: check strdup() failures and replace constant

[PATCH] libsepol: check strdup() failures and replace constant

[Jokke Hämäläinen]

Check for strdup() failures. Also replace constant 18
with a safer sizeof() use.

Signed-off-by: Unto Sten <sten.unto@gmail.com>
---
 libsepol/src/kernel_to_conf.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
index 4f84ee8b..930bafab 100644
--- a/libsepol/src/kernel_to_conf.c
+++ b/libsepol/src/kernel_to_conf.c
@@ -448,8 +448,12 @@ static int write_sids_to_conf(FILE *out, const char *const *sid_to_str,
 		if (i < num_sids) {
 			sid = (char *)sid_to_str[i];
 		} else {
-			snprintf(unknown, 18, "%s%u", "UNKNOWN", i);
+			snprintf(unknown, sizeof(unknown), "%s%u", "UNKNOWN", i);
 			sid = strdup(unknown);
+			if (!sid) {
+				rc = -1;
+				goto exit;
+			}
 		}
 		rc = strs_add_at_index(strs, sid, i);
 		if (rc != 0) {
@@ -792,6 +796,10 @@ static int write_sensitivity_rules_to_conf(FILE *out, struct policydb *pdb)
 			j = level->level->sens - 1;
 			if (!sens_alias_map[j]) {
 				sens_alias_map[j] = strdup(name);
+				if (!sens_alias_map[j]) {
+					rc = -1;
+					goto exit;
+				}
 			} else {
 				alias = sens_alias_map[j];
 				sens_alias_map[j] = create_str("%s %s", 2, alias, name);
@@ -919,6 +927,10 @@ static int write_category_rules_to_conf(FILE *out, struct policydb *pdb)
 			j = cat->s.value - 1;
 			if (!cat_alias_map[j]) {
 				cat_alias_map[j] = strdup(name);
+				if (!cat_alias_map[j]) {
+					rc = -1;
+					goto exit;
+				}
 			} else {
 				alias = cat_alias_map[j];
 				cat_alias_map[j] = create_str("%s %s", 2, alias, name);
@@ -2364,7 +2376,7 @@ static int write_sid_context_rules_to_conf(FILE *out, struct policydb *pdb, cons
 		if (i < num_sids) {
 			sid = (char *)sid_to_str[i];
 		} else {
-			snprintf(unknown, 18, "%s%u", "UNKNOWN", i);
+			snprintf(unknown, sizeof(unknown), "%s%u", "UNKNOWN", i);
 			sid = unknown;
 		}
 
-- 
2.21.0

Re: [PATCH] libsepol: check strdup() failures and replace constant

[William Roberts]

On Fri, Jun 14, 2019 at 4:17 PM Jokke Hämäläinen
<jokke.hamalainen@kolttonen.fi> wrote:
>
>
> Check for strdup() failures. Also replace constant 18
> with a safer sizeof() use.

When ever you do "also" and "and" in a patch description, that's usually
an indication it should be 2 separate patches. The only case where this
is generally not followed is when both patches modify the same hunk.
Please split this, one for strdup() checks and 1 for the sizeof() usage.

>
> Signed-off-by: Unto Sten <sten.unto@gmail.com>
> ---
>  libsepol/src/kernel_to_conf.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
> index 4f84ee8b..930bafab 100644
> --- a/libsepol/src/kernel_to_conf.c
> +++ b/libsepol/src/kernel_to_conf.c
> @@ -448,8 +448,12 @@ static int write_sids_to_conf(FILE *out, const char *const *sid_to_str,
>                 if (i < num_sids) {
>                         sid = (char *)sid_to_str[i];
>                 } else {
> -                       snprintf(unknown, 18, "%s%u", "UNKNOWN", i);
> +                       snprintf(unknown, sizeof(unknown), "%s%u", "UNKNOWN", i);
>                         sid = strdup(unknown);
> +                       if (!sid) {
> +                               rc = -1;
> +                               goto exit;
> +                       }
>                 }
>                 rc = strs_add_at_index(strs, sid, i);
>                 if (rc != 0) {
> @@ -792,6 +796,10 @@ static int write_sensitivity_rules_to_conf(FILE *out, struct policydb *pdb)
>                         j = level->level->sens - 1;
>                         if (!sens_alias_map[j]) {
>                                 sens_alias_map[j] = strdup(name);
> +                               if (!sens_alias_map[j]) {
> +                                       rc = -1;
> +                                       goto exit;
> +                               }
>                         } else {
>                                 alias = sens_alias_map[j];
>                                 sens_alias_map[j] = create_str("%s %s", 2, alias, name);
> @@ -919,6 +927,10 @@ static int write_category_rules_to_conf(FILE *out, struct policydb *pdb)
>                         j = cat->s.value - 1;
>                         if (!cat_alias_map[j]) {
>                                 cat_alias_map[j] = strdup(name);
> +                               if (!cat_alias_map[j]) {
> +                                       rc = -1;
> +                                       goto exit;
> +                               }
>                         } else {
>                                 alias = cat_alias_map[j];
>                                 cat_alias_map[j] = create_str("%s %s", 2, alias, name);
> @@ -2364,7 +2376,7 @@ static int write_sid_context_rules_to_conf(FILE *out, struct policydb *pdb, cons
>                 if (i < num_sids) {
>                         sid = (char *)sid_to_str[i];
>                 } else {
> -                       snprintf(unknown, 18, "%s%u", "UNKNOWN", i);
> +                       snprintf(unknown, sizeof(unknown), "%s%u", "UNKNOWN", i);
>                         sid = unknown;
>                 }
>
> --
> 2.21.0
>

Re: [PATCH] libsepol: check strdup() failures and replace constant

[Jokke Hämäläinen]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]


Hello,

On Sun, 16 Jun 2019, William Roberts wrote:

> When ever you do "also" and "and" in a patch description, that's usually
> an indication it should be 2 separate patches. The only case where this
> is generally not followed is when both patches modify the same hunk.
> Please split this, one for strdup() checks and 1 for the sizeof() usage.

Okay, done and reposted. Stephen only asked me to add the signed-off-by 
line so I figured the patch was okay as is.

Best regards,
Jokke Hämäläinen