Skip to content

Commit e163fdb

Browse files
committed
pstore/ram: Regularize prz label allocation lifetime
In my attempt to fix a memory leak, I introduced a double-free in the pstore error path. Instead of trying to manage the allocation lifetime between persistent_ram_new() and its callers, adjust the logic so persistent_ram_new() always takes a kstrdup() copy, and leaves the caller's allocation lifetime up to the caller. Therefore callers are _always_ responsible for freeing their label. Before, it only needed freeing when the prz itself failed to allocate, and not in any of the other prz failure cases, which callers would have no visibility into, which is the root design problem that lead to both the leak and now double-free bugs. Reported-by: Cengiz Can <[email protected]> Link: https://lore.kernel.org/lkml/[email protected] Fixes: 8df955a ("pstore/ram: Fix error-path memory leak in persistent_ram_new() callers") Cc: [email protected] Signed-off-by: Kees Cook <[email protected]>
1 parent 9e5f1c1 commit e163fdb

File tree

2 files changed

+3
-3
lines changed

2 files changed

+3
-3
lines changed

fs/pstore/ram.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -583,12 +583,12 @@ static int ramoops_init_przs(const char *name,
583583
prz_ar[i] = persistent_ram_new(*paddr, zone_sz, sig,
584584
&cxt->ecc_info,
585585
cxt->memtype, flags, label);
586+
kfree(label);
586587
if (IS_ERR(prz_ar[i])) {
587588
err = PTR_ERR(prz_ar[i]);
588589
dev_err(dev, "failed to request %s mem region (0x%zx@0x%llx): %d\n",
589590
name, record_size,
590591
(unsigned long long)*paddr, err);
591-
kfree(label);
592592

593593
while (i > 0) {
594594
i--;
@@ -629,12 +629,12 @@ static int ramoops_init_prz(const char *name,
629629
label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
630630
*prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info,
631631
cxt->memtype, PRZ_FLAG_ZAP_OLD, label);
632+
kfree(label);
632633
if (IS_ERR(*prz)) {
633634
int err = PTR_ERR(*prz);
634635

635636
dev_err(dev, "failed to request %s mem region (0x%zx@0x%llx): %d\n",
636637
name, sz, (unsigned long long)*paddr, err);
637-
kfree(label);
638638
return err;
639639
}
640640

fs/pstore/ram_core.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size,
574574
/* Initialize general buffer state. */
575575
raw_spin_lock_init(&prz->buffer_lock);
576576
prz->flags = flags;
577-
prz->label = label;
577+
prz->label = kstrdup(label, GFP_KERNEL);
578578

579579
ret = persistent_ram_buffer_map(start, size, prz, memtype);
580580
if (ret)

0 commit comments

Comments
 (0)