| Message ID | alpine.LFD.2.21.1906150215140.18365@34-41-5D-CA-59-C7 (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libsepol: check strdup() failures and replace constant | expand |
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 >
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
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; }
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(-)