| From 2d07d7882edcbec1d7c263f818720d3b5ae87596 Mon Sep 17 00:00:00 2001 |
| From: Kees Cook <keescook@chromium.org> |
| Date: Wed, 8 Jan 2020 10:06:54 -0800 |
| Subject: [PATCH] pstore/ram: Regularize prz label allocation lifetime |
| |
| commit e163fdb3f7f8c62dccf194f3f37a7bcb3c333aa8 upstream. |
| |
| 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 <cengiz@kernel.wtf> |
| Link: https://lore.kernel.org/lkml/d4ec59002ede4aaf9928c7f7526da87c@kernel.wtf |
| Fixes: 8df955a32a73 ("pstore/ram: Fix error-path memory leak in persistent_ram_new() callers") |
| Cc: stable@vger.kernel.org |
| Signed-off-by: Kees Cook <keescook@chromium.org> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c |
| index 5f3753664269..2c549b5dd4aa 100644 |
| --- a/fs/pstore/ram.c |
| +++ b/fs/pstore/ram.c |
| @@ -583,12 +583,12 @@ static int ramoops_init_przs(const char *name, |
| prz_ar[i] = persistent_ram_new(*paddr, zone_sz, sig, |
| &cxt->ecc_info, |
| cxt->memtype, flags, label); |
| + kfree(label); |
| if (IS_ERR(prz_ar[i])) { |
| err = PTR_ERR(prz_ar[i]); |
| dev_err(dev, "failed to request %s mem region (0x%zx@0x%llx): %d\n", |
| name, record_size, |
| (unsigned long long)*paddr, err); |
| - kfree(label); |
| |
| while (i > 0) { |
| i--; |
| @@ -629,12 +629,12 @@ static int ramoops_init_prz(const char *name, |
| label = kasprintf(GFP_KERNEL, "ramoops:%s", name); |
| *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info, |
| cxt->memtype, PRZ_FLAG_ZAP_OLD, label); |
| + kfree(label); |
| if (IS_ERR(*prz)) { |
| int err = PTR_ERR(*prz); |
| |
| dev_err(dev, "failed to request %s mem region (0x%zx@0x%llx): %d\n", |
| name, sz, (unsigned long long)*paddr, err); |
| - kfree(label); |
| return err; |
| } |
| |
| diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c |
| index 8823f65888f0..1f4d8c06f9be 100644 |
| --- a/fs/pstore/ram_core.c |
| +++ b/fs/pstore/ram_core.c |
| @@ -574,7 +574,7 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, |
| /* Initialize general buffer state. */ |
| raw_spin_lock_init(&prz->buffer_lock); |
| prz->flags = flags; |
| - prz->label = label; |
| + prz->label = kstrdup(label, GFP_KERNEL); |
| |
| ret = persistent_ram_buffer_map(start, size, prz, memtype); |
| if (ret) |
| -- |
| 2.7.4 |
| |