On Mon, Nov 20, 2023 at 1:12 PM James Carter <jwcart2@xxxxxxxxx> wrote: > > On Mon, Nov 20, 2023 at 10:48 AM Christian Göttsche > <cgzones@xxxxxxxxxxxxxx> wrote: > > > > An extended access vector rule can consist of many individual ranges of > > permissions. Use a dynamically growing sized buffer for formatting such > > rules instead of a static buffer to avoid write failures due to > > truncations. > > > > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx> > > For these two patches: > Acked-by: James Carter <jwcart2@xxxxxxxxx> > These two patches have been merged. Thanks, Jim > > --- > > v2: > > reset in_range on retry > > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx> > > --- > > checkpolicy/test/dismod.c | 9 +++++- > > checkpolicy/test/dispol.c | 10 +++++- > > libsepol/src/assertion.c | 7 ++++- > > libsepol/src/kernel_to_conf.c | 9 +++--- > > libsepol/src/util.c | 57 +++++++++++++++++++++++------------ > > 5 files changed, 66 insertions(+), 26 deletions(-) > > > > diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c > > index fa7117f5..9f4a669b 100644 > > --- a/checkpolicy/test/dismod.c > > +++ b/checkpolicy/test/dismod.c > > @@ -347,6 +347,7 @@ static int display_avrule(avrule_t * avrule, policydb_t * policy, > > display_id(policy, fp, SYM_TYPES, avrule->perms->data - 1, ""); > > } else if (avrule->specified & AVRULE_XPERMS) { > > avtab_extended_perms_t xperms; > > + char *perms; > > int i; > > > > if (avrule->xperms->specified == AVRULE_XPERMS_IOCTLFUNCTION) > > @@ -362,7 +363,13 @@ static int display_avrule(avrule_t * avrule, policydb_t * policy, > > for (i = 0; i < EXTENDED_PERMS_LEN; i++) > > xperms.perms[i] = avrule->xperms->perms[i]; > > > > - fprintf(fp, "%s", sepol_extended_perms_to_string(&xperms)); > > + perms = sepol_extended_perms_to_string(&xperms); > > + if (!perms) { > > + fprintf(fp, " ERROR: failed to format xperms\n"); > > + return -1; > > + } > > + fprintf(fp, "%s", perms); > > + free(perms); > > } > > > > fprintf(fp, ";\n"); > > diff --git a/checkpolicy/test/dispol.c b/checkpolicy/test/dispol.c > > index b567ce77..944ef7ec 100644 > > --- a/checkpolicy/test/dispol.c > > +++ b/checkpolicy/test/dispol.c > > @@ -196,6 +196,8 @@ static int render_av_rule(avtab_key_t * key, avtab_datum_t * datum, uint32_t wha > > fprintf(fp, ";\n"); > > } > > } else if (key->specified & AVTAB_XPERMS) { > > + char *perms; > > + > > if (key->specified & AVTAB_XPERMS_ALLOWED) > > fprintf(fp, "allowxperm "); > > else if (key->specified & AVTAB_XPERMS_AUDITALLOW) > > @@ -203,7 +205,13 @@ static int render_av_rule(avtab_key_t * key, avtab_datum_t * datum, uint32_t wha > > else if (key->specified & AVTAB_XPERMS_DONTAUDIT) > > fprintf(fp, "dontauditxperm "); > > render_key(key, p, fp); > > - fprintf(fp, "%s;\n", sepol_extended_perms_to_string(datum->xperms)); > > + perms = sepol_extended_perms_to_string(datum->xperms); > > + if (!perms) { > > + fprintf(fp, " ERROR: failed to format xperms\n"); > > + return -1; > > + } > > + fprintf(fp, "%s;\n", perms); > > + free(perms); > > } else { > > fprintf(fp, " ERROR: no valid rule type specified\n"); > > return -1; > > diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c > > index b6ac4cfe..6de7d031 100644 > > --- a/libsepol/src/assertion.c > > +++ b/libsepol/src/assertion.c > > @@ -178,15 +178,20 @@ static int report_assertion_extended_permissions(sepol_handle_t *handle, > > rc = check_extended_permissions(avrule->xperms, xperms); > > /* failure on the extended permission check_extended_permissions */ > > if (rc) { > > + char *permstring; > > + > > extended_permissions_violated(&error, avrule->xperms, xperms); > > + permstring = sepol_extended_perms_to_string(&error); > > + > > ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of %s) violated by\n" > > "allowxperm %s %s:%s %s;", > > avrule->source_line, avrule->source_filename, avrule->line, policy_name(p), > > p->p_type_val_to_name[i], > > p->p_type_val_to_name[j], > > p->p_class_val_to_name[curperm->tclass - 1], > > - sepol_extended_perms_to_string(&error)); > > + permstring ?: "<format-failure>"); > > > > + free(permstring); > > errors++; > > } > > } > > diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c > > index b0ae16d9..b5b530d6 100644 > > --- a/libsepol/src/kernel_to_conf.c > > +++ b/libsepol/src/kernel_to_conf.c > > @@ -1683,7 +1683,7 @@ static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_dat > > uint32_t data = datum->data; > > type_datum_t *type; > > const char *flavor, *src, *tgt, *class, *perms, *new; > > - char *rule = NULL; > > + char *rule = NULL, *permstring; > > > > switch (0xFFF & key->specified) { > > case AVTAB_ALLOWED: > > @@ -1738,13 +1738,14 @@ static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_dat > > rule = create_str("%s %s %s:%s { %s };", 5, > > flavor, src, tgt, class, perms+1); > > } else if (key->specified & AVTAB_XPERMS) { > > - perms = sepol_extended_perms_to_string(datum->xperms); > > - if (perms == NULL) { > > + permstring = sepol_extended_perms_to_string(datum->xperms); > > + if (permstring == NULL) { > > ERR(NULL, "Failed to generate extended permission string"); > > goto exit; > > } > > > > - rule = create_str("%s %s %s:%s %s;", 5, flavor, src, tgt, class, perms); > > + rule = create_str("%s %s %s:%s %s;", 5, flavor, src, tgt, class, permstring); > > + free(permstring); > > } else { > > new = pdb->p_type_val_to_name[data - 1]; > > > > diff --git a/libsepol/src/util.c b/libsepol/src/util.c > > index 0a2edc85..2f877920 100644 > > --- a/libsepol/src/util.c > > +++ b/libsepol/src/util.c > > @@ -132,21 +132,32 @@ char *sepol_extended_perms_to_string(avtab_extended_perms_t *xperms) > > uint16_t low_bit; > > uint16_t low_value; > > unsigned int bit; > > - unsigned int in_range = 0; > > - static char xpermsbuf[2048]; > > - char *p; > > - int len, xpermslen = 0; > > - xpermsbuf[0] = '\0'; > > - p = xpermsbuf; > > + unsigned int in_range; > > + char *buffer = NULL, *p; > > + int len; > > + size_t remaining, size = 128; > > > > if ((xperms->specified != AVTAB_XPERMS_IOCTLFUNCTION) > > && (xperms->specified != AVTAB_XPERMS_IOCTLDRIVER)) > > return NULL; > > > > - len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "ioctl { "); > > +retry: > > + size *= 2; > > + if (size == 0) > > + goto err; > > + p = realloc(buffer, size); > > + if (!p) > > + goto err; > > + buffer = p; > > + remaining = size; > > + > > + len = snprintf(p, remaining, "ioctl { "); > > + if (len < 0 || (size_t)len >= remaining) > > + goto err; > > p += len; > > - xpermslen += len; > > + remaining -= len; > > > > + in_range = 0; > > for (bit = 0; bit < sizeof(xperms->perms)*8; bit++) { > > if (!xperm_test(bit, xperms->perms)) > > continue; > > @@ -165,35 +176,43 @@ char *sepol_extended_perms_to_string(avtab_extended_perms_t *xperms) > > value = xperms->driver<<8 | bit; > > if (in_range) { > > low_value = xperms->driver<<8 | low_bit; > > - len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx-0x%hx ", low_value, value); > > + len = snprintf(p, remaining, "0x%hx-0x%hx ", low_value, value); > > } else { > > - len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx ", value); > > + len = snprintf(p, remaining, "0x%hx ", value); > > } > > } else if (xperms->specified & AVTAB_XPERMS_IOCTLDRIVER) { > > value = bit << 8; > > if (in_range) { > > low_value = low_bit << 8; > > - len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx-0x%hx ", low_value, (uint16_t) (value|0xff)); > > + len = snprintf(p, remaining, "0x%hx-0x%hx ", low_value, (uint16_t) (value|0xff)); > > } else { > > - len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx-0x%hx ", value, (uint16_t) (value|0xff)); > > + len = snprintf(p, remaining, "0x%hx-0x%hx ", value, (uint16_t) (value|0xff)); > > } > > > > } > > > > - if (len < 0 || (size_t) len >= (sizeof(xpermsbuf) - xpermslen)) > > - return NULL; > > + if (len < 0) > > + goto err; > > + if ((size_t) len >= remaining) > > + goto retry; > > > > p += len; > > - xpermslen += len; > > + remaining -= len; > > if (in_range) > > in_range = 0; > > } > > > > - len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "}"); > > - if (len < 0 || (size_t) len >= (sizeof(xpermsbuf) - xpermslen)) > > - return NULL; > > + len = snprintf(p, remaining, "}"); > > + if (len < 0) > > + goto err; > > + if ((size_t) len >= remaining) > > + goto retry; > > + > > + return buffer; > > > > - return xpermsbuf; > > +err: > > + free(buffer); > > + return NULL; > > } > > > > /* > > -- > > 2.42.0 > >