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 <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>

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 487ee39b438a75f12631654e3b6cae6aa4f65cff..013486b5125ed78f80f10024c7c1f3d67fe8d647 100644 (file)
--- 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 8823f65888f034cd5997a68898f2c7792d8ae69c..1f4d8c06f9be61d8f95747aa1d1b2dd730d0a36d 100644 (file)
--- 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)