Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Memory leak of enum label string when it contains invalid characters. --
--- Patch [4]: [PATCH v11 4/4] firmware: coreboot: Add CFR firmware attributes driver --- commit ea61591b8dd6ac619938eac5285db7c41217e27e Author: Sean Rhodes <[email protected]> firmware: coreboot: Add CFR firmware attributes driver This commit adds a firmware-attributes driver for the coreboot CFR table entry to describe firmware setup options. It exposes runtime EFI variable-backed boolean, enum, and integer settings through sysfs. Link: https://doc.coreboot.org/drivers/cfr.html Link: https://doc.coreboot.org/drivers/cfr_internal.html > diff --git a/drivers/firmware/coreboot/coreboot-cfr.c > b/drivers/firmware/coreboot/coreboot-cfr.c > new file mode 100644 > index 0000000000000..1dfb2d29046b6 > --- /dev/null > +++ b/drivers/firmware/coreboot/coreboot-cfr.c [ ... ] > +static int coreboot_cfr_copy_enum_values(struct coreboot_cfr_setting > *setting, > + const void *base, size_t len) > +{ [ ... ] > + for (;;) { > + entry = coreboot_cfr_next_entry(&iterator); [ ... ] > + value = &setting->values[setting->n_values]; > + value->label = coreboot_cfr_string_dup(label); > + if (!value->label) > + return -ENOMEM; > + if (!coreboot_cfr_string_is_valid_label(value->label)) > + return -EINVAL; [Severity: Medium] Does this code leak the newly allocated value->label string? When coreboot_cfr_string_is_valid_label() fails and returns -EINVAL, setting->n_values has not been incremented yet. If the caller then cleans up the error by calling coreboot_cfr_free_setting(), the cleanup loop: static void coreboot_cfr_free_setting(struct coreboot_cfr_setting *setting) { unsigned int i; for (i = 0; i < setting->n_values; i++) kfree(setting->values[i].label); ... will skip freeing the string that was just allocated because setting->n_values does not include it yet. Should kfree() be called on value->label before returning -EINVAL here? > + > + value->value = enum_value->value; > + setting->n_values++; > + } > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
