lkml.org 
[lkml]   [2016]   [Aug]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] pstore/ramoops: fixup driver removal
Date
A basic rmmod ramoops segfaults. Let's see why.

Since commit 34f0ec82e0a9 ("pstore: Correct the max_dump_cnt clearing of
ramoops") sets ->max_dump_cnt to zero before looping over ->przs but we
didn't use it before that either.

And since commit ee1d267423a1 ("pstore: add pstore unregister") we free
that memory on rmmod.

But even then, we looped until a NULL pointer or ERR. I don't see where
it is ensured that the last member is NULL. Let's try this instead:
simply error recovery and free. Clean up in error case where ressouces
were allocated. And then, in the free path, rely on ->max_dump_cnt in
the free path.

Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tony Luck <tony.luck@intel.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---

Not sure if I miss something obvious or not.

fs/pstore/ram.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 7a034d62cf8c..2340262a7e97 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -377,13 +377,14 @@ static void ramoops_free_przs(struct ramoops_context *cxt)
{
int i;

- cxt->max_dump_cnt = 0;
if (!cxt->przs)
return;

- for (i = 0; !IS_ERR_OR_NULL(cxt->przs[i]); i++)
+ for (i = 0; i < cxt->max_dump_cnt; i++)
persistent_ram_free(cxt->przs[i]);
+
kfree(cxt->przs);
+ cxt->max_dump_cnt = 0;
}

static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt,
@@ -408,7 +409,7 @@ static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt,
GFP_KERNEL);
if (!cxt->przs) {
dev_err(dev, "failed to initialize a prz array for dumps\n");
- goto fail_prz;
+ goto fail_mem;
}

for (i = 0; i < cxt->max_dump_cnt; i++) {
@@ -419,6 +420,11 @@ static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt,
err = PTR_ERR(cxt->przs[i]);
dev_err(dev, "failed to request mem region (0x%zx@0x%llx): %d\n",
cxt->record_size, (unsigned long long)*paddr, err);
+
+ while (i > 0) {
+ i--;
+ persistent_ram_free(cxt->przs[i]);
+ }
goto fail_prz;
}
*paddr += cxt->record_size;
@@ -426,7 +432,9 @@ static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt,

return 0;
fail_prz:
- ramoops_free_przs(cxt);
+ kfree(cxt->przs);
+fail_mem:
+ cxt->max_dump_cnt = 0;
return err;
}

@@ -659,7 +667,6 @@ static int ramoops_remove(struct platform_device *pdev)
struct ramoops_context *cxt = &oops_cxt;

pstore_unregister(&cxt->pstore);
- cxt->max_dump_cnt = 0;

kfree(cxt->pstore.buf);
cxt->pstore.bufsize = 0;
--
2.9.3
\
 
 \ /
  Last update: 2016-09-17 09:57    [W:0.048 / U:0.280 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site