Patch "pstore/ramoops: fixup driver removal" has been added to the 4.4-stable tree

This is a note to let you know that I've just added the patch titled

pstore/ramoops: fixup driver removal

to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
pstore-ramoops-fixup-driver-removal.patch
and it can be found in the queue-4.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From: Sebastian Andrzej Siewior <[email protected]>
Date: Thu, 8 Sep 2016 13:48:05 +0200
Subject: pstore/ramoops: fixup driver removal

From: Sebastian Andrzej Siewior <[email protected]>

commit 4407de74df18ed405cc5998990004c813ccfdbde upstream.

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 resources were allocated. And then, in the free path, rely on ->max_dump_cnt in the free path.

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

@@ -375,13 +375,14 @@ static void ramoops_free_przs(struct ram
 {
 	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,

@@ -406,7 +407,7 @@ static int ramoops_init_przs(struct devi
 			     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++) {

@@ -417,6 +418,11 @@ static int ramoops_init_przs(struct devi
 			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;

@@ -424,7 +430,9 @@ static int ramoops_init_przs(struct devi
 
 	return 0;
 fail_prz:
-	ramoops_free_przs(cxt);
+	kfree(cxt->przs);
+fail_mem:
+	cxt->max_dump_cnt = 0;
 	return err;
 }
 

@@ -583,7 +591,6 @@ static int ramoops_remove(struct platfor
 	struct ramoops_context *cxt = &oops_cxt;
 
 	pstore_unregister(&cxt->pstore);
-	cxt->max_dump_cnt = 0;
 
 	kfree(cxt->pstore.buf);
 	cxt->pstore.bufsize = 0;

Patches currently in stable-queue which might be from [email protected] are

queue-4.4/pstore-core-drop-cmpxchg-based-updates.patch queue-4.4/pstore-ramoops-fixup-driver-removal.patch