[PATCH pstore-next v2 0/4] Refactor compression initialization

[PATCH pstore-next v2 0/4] Refactor compression initialization

[Kees Cook]

Sai noticed pstore wasn't catching early crashes, so Joel fixed it
by moving init earlier again (with compression init done later). I
refactored things a bit to avoid modular build problems noticed by
Guenter. And while doing this work, I also improved /proc/iomem labels
after Dan helped me get NVDIMM working so I could meaningfully test the
very early crash handling.

Thanks everyone!

-Kees

v2:
- add init refactoring to avoid modular build failures.


Joel Fernandes (Google) (1):
  pstore: Allocate compression during late_initcall()

Kees Cook (3):
  pstore: Centralize init/exit routines
  pstore: Refactor compression initialization
  pstore/ram: Clarify resource reservation labels

 fs/pstore/inode.c          | 11 +-----
 fs/pstore/internal.h       |  5 ++-
 fs/pstore/platform.c       | 75 +++++++++++++++++++++++++++++++-------
 fs/pstore/ram.c            | 18 +++++++--
 fs/pstore/ram_core.c       | 11 ++++--
 include/linux/pstore_ram.h |  3 +-
 6 files changed, 90 insertions(+), 33 deletions(-)

-- 
2.17.1

[PATCH pstore-next v2 1/4] pstore: Centralize init/exit routines

[Kees Cook]

In preparation for having additional actions during init/exit, this moves
the init/exit into platform.c, centralizing the logic to make call outs
to the fs init/exit.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/pstore/inode.c    | 11 ++---------
 fs/pstore/internal.h |  5 +++--
 fs/pstore/platform.c | 23 +++++++++++++++++++++++
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 5fcb845b9fec..8cf2218b46a7 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -482,12 +482,10 @@ static struct file_system_type pstore_fs_type = {
 	.kill_sb	= pstore_kill_sb,
 };
 
-static int __init init_pstore_fs(void)
+int __init pstore_init_fs(void)
 {
 	int err;
 
-	pstore_choose_compression();
-
 	/* Create a convenient mount point for people to access pstore */
 	err = sysfs_create_mount_point(fs_kobj, "pstore");
 	if (err)
@@ -500,14 +498,9 @@ static int __init init_pstore_fs(void)
 out:
 	return err;
 }
-module_init(init_pstore_fs)
 
-static void __exit exit_pstore_fs(void)
+void __exit pstore_exit_fs(void)
 {
 	unregister_filesystem(&pstore_fs_type);
 	sysfs_remove_mount_point(fs_kobj, "pstore");
 }
-module_exit(exit_pstore_fs)
-
-MODULE_AUTHOR("Tony Luck <tony.luck@intel.com>");
-MODULE_LICENSE("GPL");
diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h
index fb767e28aeb2..98bb4fa6e6a7 100644
--- a/fs/pstore/internal.h
+++ b/fs/pstore/internal.h
@@ -37,7 +37,8 @@ extern bool	pstore_is_mounted(void);
 extern void	pstore_record_init(struct pstore_record *record,
 				   struct pstore_info *psi);
 
-/* Called during module_init() */
-extern void __init pstore_choose_compression(void);
+/* Called during pstore init/exit. */
+int __init	pstore_init_fs(void);
+void __init	pstore_exit_fs(void);
 
 #endif
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 15e99d5a681d..d61e26812af6 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -780,8 +780,31 @@ void __init pstore_choose_compression(void)
 	}
 }
 
+static int __init pstore_init(void)
+{
+	int ret;
+
+	pstore_choose_compression();
+
+	ret = pstore_init_fs();
+	if (ret)
+		return ret;
+
+	return 0;
+}
+module_init(pstore_init)
+
+static void __exit pstore_exit(void)
+{
+	pstore_exit_fs();
+}
+module_exit(pstore_exit)
+
 module_param(compress, charp, 0444);
 MODULE_PARM_DESC(compress, "Pstore compression to use");
 
 module_param(backend, charp, 0444);
 MODULE_PARM_DESC(backend, "Pstore backend to use");
+
+MODULE_AUTHOR("Tony Luck <tony.luck@intel.com>");
+MODULE_LICENSE("GPL");
-- 
2.17.1

[PATCH pstore-next v2 2/4] pstore: Allocate compression during late_initcall()

[Kees Cook]

From: "Joel Fernandes (Google)" <joel@joelfernandes.org>

ramoops's call of pstore_register() was recently moved to run during
late_initcall() because the crypto backend may not have been ready during
postcore_initcall(). This meant early-boot crash dumps were not getting
caught by pstore any more.

Instead, lets allow calls to pstore_register() earlier, and once crypto
is ready we can initialize the compression.

Reported-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Fixes: cb3bee0369bc ("pstore: Use crypto compress API")
[kees: trivial rebase]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/pstore/platform.c | 10 +++++++++-
 fs/pstore/ram.c      |  2 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index d61e26812af6..578f178a695f 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -786,13 +786,21 @@ static int __init pstore_init(void)
 
 	pstore_choose_compression();
 
+	/*
+	 * Check if any pstore backends registered earlier but did not
+	 * initialize compression because crypto was not ready. If so,
+	 * initialize compression now.
+	 */
+	if (psinfo && !tfm)
+		allocate_buf_for_compression();
+
 	ret = pstore_init_fs();
 	if (ret)
 		return ret;
 
 	return 0;
 }
-module_init(pstore_init)
+late_initcall(pstore_init);
 
 static void __exit pstore_exit(void)
 {
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index f4fd2e72add4..6ea9cd801044 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -962,7 +962,7 @@ static int __init ramoops_init(void)
 
 	return ret;
 }
-late_initcall(ramoops_init);
+postcore_initcall(ramoops_init);
 
 static void __exit ramoops_exit(void)
 {
-- 
2.17.1

[PATCH pstore-next v2 3/4] pstore: Refactor compression initialization

[Kees Cook]

This refactors compression initialization slightly to better handle
getting potentially called twice (via early pstore_register() calls
and later pstore_init()) and improves the comments and reporting to be
more verbose.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/pstore/platform.c | 48 ++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 578f178a695f..b821054ca3ed 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -274,36 +274,56 @@ static int pstore_decompress(void *in, void *out,
 
 static void allocate_buf_for_compression(void)
 {
+	struct crypto_comp *ctx;
+	int size;
+	char *buf;
+
+	/* Skip if not built-in or compression backend not selected yet. */
 	if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS) || !zbackend)
 		return;
 
+	/* Skip if no pstore backend yet or compression init already done. */
+	if (!psinfo || tfm)
+		return;
+
 	if (!crypto_has_comp(zbackend->name, 0, 0)) {
-		pr_err("No %s compression\n", zbackend->name);
+		pr_err("Unknown compression: %s\n", zbackend->name);
 		return;
 	}
 
-	big_oops_buf_sz = zbackend->zbufsize(psinfo->bufsize);
-	if (big_oops_buf_sz <= 0)
+	size = zbackend->zbufsize(psinfo->bufsize);
+	if (size <= 0) {
+		pr_err("Invalid compression size for %s: %d\n",
+		       zbackend->name, size);
 		return;
+	}
 
-	big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL);
-	if (!big_oops_buf) {
-		pr_err("allocate compression buffer error!\n");
+	buf = kmalloc(size, GFP_KERNEL);
+	if (!buf) {
+		pr_err("Failed %d byte compression buffer allocation for: %s\n",
+		       size, zbackend->name);
 		return;
 	}
 
-	tfm = crypto_alloc_comp(zbackend->name, 0, 0);
-	if (IS_ERR_OR_NULL(tfm)) {
-		kfree(big_oops_buf);
-		big_oops_buf = NULL;
-		pr_err("crypto_alloc_comp() failed!\n");
+	ctx = crypto_alloc_comp(zbackend->name, 0, 0);
+	if (IS_ERR_OR_NULL(ctx)) {
+		kfree(buf);
+		pr_err("crypto_alloc_comp('%s') failed: %ld\n", zbackend->name,
+		       PTR_ERR(ctx));
 		return;
 	}
+
+	/* A non-NULL big_oops_buf indicates compression is available. */
+	tfm = ctx;
+	big_oops_buf_sz = size;
+	big_oops_buf = buf;
+
+	pr_info("Using compression: %s\n", zbackend->name);
 }
 
 static void free_buf_for_compression(void)
 {
-	if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && !IS_ERR_OR_NULL(tfm))
+	if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && tfm)
 		crypto_free_comp(tfm);
 	kfree(big_oops_buf);
 	big_oops_buf = NULL;
@@ -774,7 +794,6 @@ void __init pstore_choose_compression(void)
 	for (step = zbackends; step->name; step++) {
 		if (!strcmp(compress, step->name)) {
 			zbackend = step;
-			pr_info("using %s compression\n", zbackend->name);
 			return;
 		}
 	}
@@ -791,8 +810,7 @@ static int __init pstore_init(void)
 	 * initialize compression because crypto was not ready. If so,
 	 * initialize compression now.
 	 */
-	if (psinfo && !tfm)
-		allocate_buf_for_compression();
+	allocate_buf_for_compression();
 
 	ret = pstore_init_fs();
 	if (ret)
-- 
2.17.1

[PATCH pstore-next v2 4/4] pstore/ram: Clarify resource reservation labels

[Kees Cook]

When ramoops reserved a memory region in the kernel, it had an unhelpful
label of "persistent_memory". When reading /proc/iomem, it would be
repeated many times, did not hint that it was ramoops in particular,
and didn't clarify very much about what each was used for:

400000000-407ffffff : Persistent Memory (legacy)
  400000000-400000fff : persistent_memory
  400001000-400001fff : persistent_memory
...
  4000ff000-4000fffff : persistent_memory

Instead, this adds meaningful labels for how the various regions are
being used:

400000000-407ffffff : Persistent Memory (legacy)
  400000000-400000fff : ramoops:dump(0/252)
  400001000-400001fff : ramoops:dump(1/252)
...
  4000fc000-4000fcfff : ramoops:dump(252/252)
  4000fd000-4000fdfff : ramoops:console
  4000fe000-4000fe3ff : ramoops:ftrace(0/3)
  4000fe400-4000fe7ff : ramoops:ftrace(1/3)
  4000fe800-4000febff : ramoops:ftrace(2/3)
  4000fec00-4000fefff : ramoops:ftrace(3/3)
  4000ff000-4000fffff : ramoops:pmsg

Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 fs/pstore/ram.c            | 16 +++++++++++++---
 fs/pstore/ram_core.c       | 11 +++++++----
 include/linux/pstore_ram.h |  3 ++-
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 6ea9cd801044..ffcff6516e89 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -587,9 +587,16 @@ static int ramoops_init_przs(const char *name,
 		goto fail;
 
 	for (i = 0; i < *cnt; i++) {
+		char *label;
+
+		if (*cnt == 1)
+			label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
+		else
+			label = kasprintf(GFP_KERNEL, "ramoops:%s(%d/%d)",
+					  name, i, *cnt - 1);
 		prz_ar[i] = persistent_ram_new(*paddr, zone_sz, sig,
-						  &cxt->ecc_info,
-						  cxt->memtype, flags);
+					       &cxt->ecc_info,
+					       cxt->memtype, flags, 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",
@@ -619,6 +626,8 @@ static int ramoops_init_prz(const char *name,
 			    struct persistent_ram_zone **prz,
 			    phys_addr_t *paddr, size_t sz, u32 sig)
 {
+	char *label;
+
 	if (!sz)
 		return 0;
 
@@ -629,8 +638,9 @@ static int ramoops_init_prz(const char *name,
 		return -ENOMEM;
 	}
 
+	label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
 	*prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info,
-				  cxt->memtype, 0);
+				  cxt->memtype, 0, label);
 	if (IS_ERR(*prz)) {
 		int err = PTR_ERR(*prz);
 
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 0792595ebcfb..12e21f789194 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -438,11 +438,11 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size,
 }
 
 static void *persistent_ram_iomap(phys_addr_t start, size_t size,
-		unsigned int memtype)
+		unsigned int memtype, char *label)
 {
 	void *va;
 
-	if (!request_mem_region(start, size, "persistent_ram")) {
+	if (!request_mem_region(start, size, label ?: "ramoops")) {
 		pr_err("request mem region (0x%llx@0x%llx) failed\n",
 			(unsigned long long)size, (unsigned long long)start);
 		return NULL;
@@ -470,7 +470,8 @@ static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
 	if (pfn_valid(start >> PAGE_SHIFT))
 		prz->vaddr = persistent_ram_vmap(start, size, memtype);
 	else
-		prz->vaddr = persistent_ram_iomap(start, size, memtype);
+		prz->vaddr = persistent_ram_iomap(start, size, memtype,
+						  prz->label);
 
 	if (!prz->vaddr) {
 		pr_err("%s: Failed to map 0x%llx pages at 0x%llx\n", __func__,
@@ -541,12 +542,13 @@ void persistent_ram_free(struct persistent_ram_zone *prz)
 	prz->ecc_info.par = NULL;
 
 	persistent_ram_free_old(prz);
+	kfree(prz->label);
 	kfree(prz);
 }
 
 struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size,
 			u32 sig, struct persistent_ram_ecc_info *ecc_info,
-			unsigned int memtype, u32 flags)
+			unsigned int memtype, u32 flags, char *label)
 {
 	struct persistent_ram_zone *prz;
 	int ret = -ENOMEM;
@@ -560,6 +562,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;
 
 	ret = persistent_ram_buffer_map(start, size, prz, memtype);
 	if (ret)
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index e6d226464838..602d64725222 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -46,6 +46,7 @@ struct persistent_ram_zone {
 	phys_addr_t paddr;
 	size_t size;
 	void *vaddr;
+	char *label;
 	struct persistent_ram_buffer *buffer;
 	size_t buffer_size;
 	u32 flags;
@@ -65,7 +66,7 @@ struct persistent_ram_zone {
 
 struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size,
 			u32 sig, struct persistent_ram_ecc_info *ecc_info,
-			unsigned int memtype, u32 flags);
+			unsigned int memtype, u32 flags, char *label);
 void persistent_ram_free(struct persistent_ram_zone *prz);
 void persistent_ram_zap(struct persistent_ram_zone *prz);
 
-- 
2.17.1

Re: [PATCH pstore-next v2 0/4] Refactor compression initialization

[Guenter Roeck]

On Thu, Oct 18, 2018 at 11:56:12AM -0700, Kees Cook wrote:
> Sai noticed pstore wasn't catching early crashes, so Joel fixed it
> by moving init earlier again (with compression init done later). I
> refactored things a bit to avoid modular build problems noticed by
> Guenter. And while doing this work, I also improved /proc/iomem labels
> after Dan helped me get NVDIMM working so I could meaningfully test the
> very early crash handling.
> 
> Thanks everyone!
> 

This version fixes the compile failure. pstore also still works with the
series applied on top of v4.19-rc8; tested on a Chromebook.

Tested-by: Guenter Roeck <groeck@chromium.org>

Guenter

> -Kees
> 
> v2:
> - add init refactoring to avoid modular build failures.
> 
> 
> Joel Fernandes (Google) (1):
>   pstore: Allocate compression during late_initcall()
> 
> Kees Cook (3):
>   pstore: Centralize init/exit routines
>   pstore: Refactor compression initialization
>   pstore/ram: Clarify resource reservation labels
> 
>  fs/pstore/inode.c          | 11 +-----
>  fs/pstore/internal.h       |  5 ++-
>  fs/pstore/platform.c       | 75 +++++++++++++++++++++++++++++++-------
>  fs/pstore/ram.c            | 18 +++++++--
>  fs/pstore/ram_core.c       | 11 ++++--
>  include/linux/pstore_ram.h |  3 +-
>  6 files changed, 90 insertions(+), 33 deletions(-)
> 
> -- 
> 2.17.1
> 

Re: [PATCH pstore-next v2 0/4] Refactor compression initialization

[Kees Cook]

On Thu, Oct 18, 2018 at 3:01 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Thu, Oct 18, 2018 at 11:56:12AM -0700, Kees Cook wrote:
>> Sai noticed pstore wasn't catching early crashes, so Joel fixed it
>> by moving init earlier again (with compression init done later). I
>> refactored things a bit to avoid modular build problems noticed by
>> Guenter. And while doing this work, I also improved /proc/iomem labels
>> after Dan helped me get NVDIMM working so I could meaningfully test the
>> very early crash handling.
>>
>> Thanks everyone!
>>
>
> This version fixes the compile failure. pstore also still works with the
> series applied on top of v4.19-rc8; tested on a Chromebook.
>
> Tested-by: Guenter Roeck <groeck@chromium.org>

Awesome, thanks!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH pstore-next v2 2/4] pstore: Allocate compression during late_initcall()

[Douglas Anderson]

Hi,

On Thu, Oct 18, 2018 at 11:56 AM Kees Cook <keescook@chromium.org> wrote:
>
> From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
>
> ramoops's call of pstore_register() was recently moved to run during
> late_initcall() because the crypto backend may not have been ready during
> postcore_initcall(). This meant early-boot crash dumps were not getting
> caught by pstore any more.
>
> Instead, lets allow calls to pstore_register() earlier, and once crypto
> is ready we can initialize the compression.
>
> Reported-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> Fixes: cb3bee0369bc ("pstore: Use crypto compress API")
> [kees: trivial rebase]
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  fs/pstore/platform.c | 10 +++++++++-
>  fs/pstore/ram.c      |  2 +-
>  2 files changed, 10 insertions(+), 2 deletions(-)

I'd propose that these three patches:

95047b0519c1 pstore: Refactor compression initialization
416031653eb5 pstore: Allocate compression during late_initcall()
cb095afd4476 pstore: Centralize init/exit routines

Get sent to linux-stable.  Specifically I'll mention that 4.19 needs
it.  IMO the regression of pstore not catching early boot crashes is
pretty serious IMO.

Thanks!

-Doug

Re: [PATCH pstore-next v2 2/4] pstore: Allocate compression during late_initcall()

[Greg KH]

On Fri, May 03, 2019 at 11:37:51AM -0700, Douglas Anderson wrote:
> Hi,
> 
> On Thu, Oct 18, 2018 at 11:56 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> >
> > ramoops's call of pstore_register() was recently moved to run during
> > late_initcall() because the crypto backend may not have been ready during
> > postcore_initcall(). This meant early-boot crash dumps were not getting
> > caught by pstore any more.
> >
> > Instead, lets allow calls to pstore_register() earlier, and once crypto
> > is ready we can initialize the compression.
> >
> > Reported-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > Fixes: cb3bee0369bc ("pstore: Use crypto compress API")
> > [kees: trivial rebase]
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  fs/pstore/platform.c | 10 +++++++++-
> >  fs/pstore/ram.c      |  2 +-
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> I'd propose that these three patches:
> 
> 95047b0519c1 pstore: Refactor compression initialization
> 416031653eb5 pstore: Allocate compression during late_initcall()
> cb095afd4476 pstore: Centralize init/exit routines
> 
> Get sent to linux-stable.  Specifically I'll mention that 4.19 needs
> it.  IMO the regression of pstore not catching early boot crashes is
> pretty serious IMO.

So just those 3 commits and not this specific patch from Joel?

thanks,

greg k-h

Re: [PATCH pstore-next v2 2/4] pstore: Allocate compression during late_initcall()

[Doug Anderson]

Hi,

On Sun, May 5, 2019 at 6:16 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, May 03, 2019 at 11:37:51AM -0700, Douglas Anderson wrote:
> > Hi,
> >
> > On Thu, Oct 18, 2018 at 11:56 AM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> > >
> > > ramoops's call of pstore_register() was recently moved to run during
> > > late_initcall() because the crypto backend may not have been ready during
> > > postcore_initcall(). This meant early-boot crash dumps were not getting
> > > caught by pstore any more.
> > >
> > > Instead, lets allow calls to pstore_register() earlier, and once crypto
> > > is ready we can initialize the compression.
> > >
> > > Reported-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > > Fixes: cb3bee0369bc ("pstore: Use crypto compress API")
> > > [kees: trivial rebase]
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > ---
> > >  fs/pstore/platform.c | 10 +++++++++-
> > >  fs/pstore/ram.c      |  2 +-
> > >  2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > I'd propose that these three patches:
> >
> > 95047b0519c1 pstore: Refactor compression initialization
> > 416031653eb5 pstore: Allocate compression during late_initcall()
> > cb095afd4476 pstore: Centralize init/exit routines
> >
> > Get sent to linux-stable.  Specifically I'll mention that 4.19 needs
> > it.  IMO the regression of pstore not catching early boot crashes is
> > pretty serious IMO.
>
> So just those 3 commits and not this specific patch from Joel?

The middle commit ("pstore: Allocate compression during
late_initcall()") is ${SUBJECT} patch and the one with the "Fixes"
tag.

The first commit ("pstore: Centralize init/exit routines") is needed
to apply the middle commit.

I haven't done lots of analysis but the last commit ("pstore: Refactor
compression initialization") sure looks like it's important if you
have the middle commit.  Specifically the middle commit allocates the
compression earlier and the last commit says that it improves handling
of this situation.


Unless someone thinks otherwise, it seems best to apply all 3?


-Doug

Re: [PATCH pstore-next v2 2/4] pstore: Allocate compression during late_initcall()

[Kees Cook]

On Mon, May 6, 2019 at 8:16 AM Doug Anderson <dianders@chromium.org> wrote:
>
> On Sun, May 5, 2019 at 6:16 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, May 03, 2019 at 11:37:51AM -0700, Douglas Anderson wrote:
> > >
> > > On Thu, Oct 18, 2018 at 11:56 AM Kees Cook <keescook@chromium.org> wrote:
> > > >
> > > > From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> > > >
> > > > ramoops's call of pstore_register() was recently moved to run during
> > > > late_initcall() because the crypto backend may not have been ready during
> > > > postcore_initcall(). This meant early-boot crash dumps were not getting
> > > > caught by pstore any more.
> > > >
> > > > Instead, lets allow calls to pstore_register() earlier, and once crypto
> > > > is ready we can initialize the compression.
> > > >
> > > > Reported-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > > Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > > > Fixes: cb3bee0369bc ("pstore: Use crypto compress API")
> > > > [kees: trivial rebase]
> > > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > > ---
> > > >  fs/pstore/platform.c | 10 +++++++++-
> > > >  fs/pstore/ram.c      |  2 +-
> > > >  2 files changed, 10 insertions(+), 2 deletions(-)
> > >
> > > I'd propose that these three patches:
> > >
> > > 95047b0519c1 pstore: Refactor compression initialization
> > > 416031653eb5 pstore: Allocate compression during late_initcall()
> > > cb095afd4476 pstore: Centralize init/exit routines
> > >
> > > Get sent to linux-stable.  Specifically I'll mention that 4.19 needs
> > > it.  IMO the regression of pstore not catching early boot crashes is
> > > pretty serious IMO.
> >
> > So just those 3 commits and not this specific patch from Joel?
>
> The middle commit ("pstore: Allocate compression during
> late_initcall()") is ${SUBJECT} patch and the one with the "Fixes"
> tag.
>
> The first commit ("pstore: Centralize init/exit routines") is needed
> to apply the middle commit.
>
> I haven't done lots of analysis but the last commit ("pstore: Refactor
> compression initialization") sure looks like it's important if you
> have the middle commit.  Specifically the middle commit allocates the
> compression earlier and the last commit says that it improves handling
> of this situation.
>
>
> Unless someone thinks otherwise, it seems best to apply all 3?

At first glance, yes, but let me double-check it this morning. There
were some subtle changes that made me not initially want to backport
these, but it should be doable. :)

-- 
Kees Cook

Re: [PATCH pstore-next v2 2/4] pstore: Allocate compression during late_initcall()

[Kees Cook]

Doug said:
> > > > I'd propose that these three patches:
> > > >
> > > > 95047b0519c1 pstore: Refactor compression initialization
> > > > 416031653eb5 pstore: Allocate compression during late_initcall()
> > > > cb095afd4476 pstore: Centralize init/exit routines

Okay, confirmed. These look sufficient to me, and the resulting tree
passes my pstore tests. Greg, can you please pull these into 4.19?

Thanks!

-- 
Kees Cook

Re: [PATCH pstore-next v2 2/4] pstore: Allocate compression during late_initcall()

[Greg KH]

On Mon, May 06, 2019 at 09:05:12AM -0700, Kees Cook wrote:
> Doug said:
> > > > > I'd propose that these three patches:
> > > > >
> > > > > 95047b0519c1 pstore: Refactor compression initialization
> > > > > 416031653eb5 pstore: Allocate compression during late_initcall()
> > > > > cb095afd4476 pstore: Centralize init/exit routines
> 
> Okay, confirmed. These look sufficient to me, and the resulting tree
> passes my pstore tests. Greg, can you please pull these into 4.19?

All now queued up, thanks.

greg k-h