[PATCH 0/8] Some pstore improvements

[PATCH 0/8] Some pstore improvements

[Guilherme G. Piccoli]

Hi pstore maintainers, this is a small series with some improvements
overall. Most of them are minors, but the implicit conversion thing
is a bit more "relevant" in a sense it's more invasive and would fit
more as a "fix".

The code is based on v6.0, and it was tested with multiple compression
algorithms (zstd, deflate, lz4, lzo, 842) and two backends (ramoops and
efi_pstore) - I've used a QEMU UEFI guest and Steam Deck for this goal.

My plan is to also work some ramoops improvements (for example, related
to [0]); they're more complex so it's deferred for a second series,
specific for that.

Reviews and comments are greatly appreciated!
Thanks in advance,

Guilherme


[0] https://lore.kernel.org/lkml/a21201cf-1e5f-fed1-356d-42c83a66fa57@igalia.com/


Guilherme G. Piccoli (8):
  pstore: Improve error reporting in case of backend overlap
  pstore: Expose kmsg_bytes as a module parameter
  pstore: Inform unregistered backend names as well
  pstore: Alert on backend write error
  pstore: Fix long-term implicit conversions in the compression routines
  MAINTAINERS: Add a mailing-list for the pstore infrastructure
  efi: pstore: Follow convention for the efi-pstore backend name
  efi: pstore: Add module parameter for setting the record size

 MAINTAINERS                       |  1 +
 drivers/firmware/efi/efi-pstore.c | 17 +++++---
 fs/pstore/platform.c              | 64 ++++++++++++++++---------------
 3 files changed, 46 insertions(+), 36 deletions(-)

-- 
2.38.0

[PATCH 7/8] efi: pstore: Follow convention for the efi-pstore backend name

[Guilherme G. Piccoli]

For some reason, the efi-pstore backend name (exposed through the
pstore infrastructure) is hardcoded as "efi", whereas all the other
backends follow a kind of convention in using the module name.

Let's do it here as well, to make user's life easier (they might
use this info for unloading the module backend, for example).

Cc: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
---
 drivers/firmware/efi/efi-pstore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index 3bddc152fcd4..97a9e84840a0 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -207,7 +207,7 @@ static int efi_pstore_erase(struct pstore_record *record)
 
 static struct pstore_info efi_pstore_info = {
 	.owner		= THIS_MODULE,
-	.name		= "efi",
+	.name		= KBUILD_MODNAME,
 	.flags		= PSTORE_FLAGS_DMESG,
 	.open		= efi_pstore_open,
 	.close		= efi_pstore_close,
-- 
2.38.0

[PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Guilherme G. Piccoli]

By default, the efi-pstore backend hardcode the UEFI variable size
as 1024 bytes. That's not a big deal, but at the same time having
no way to change that in the kernel is a bit bummer for specialized
users - there is not such a limit in the UEFI specification.

So, add here a module parameter to enable advanced users to change
the UEFI record size for efi-pstore data collection.
Through empirical analysis we observed that extreme low values
(like 8 bytes) could eventually cause writing issues, so we limit
the low end to 1024 bytes (which is also the default).

Cc: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
---
 drivers/firmware/efi/efi-pstore.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index 97a9e84840a0..78c27f6a83aa 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -10,7 +10,9 @@ MODULE_IMPORT_NS(EFIVAR);
 
 #define DUMP_NAME_LEN 66
 
-#define EFIVARS_DATA_SIZE_MAX 1024
+static unsigned int record_size = 1024;
+module_param(record_size, uint, 0444);
+MODULE_PARM_DESC(record_size, "size of each pstore UEFI var (in bytes, min/default=1024)");
 
 static bool efivars_pstore_disable =
 	IS_ENABLED(CONFIG_EFI_VARS_PSTORE_DEFAULT_DISABLE);
@@ -30,7 +32,7 @@ static int efi_pstore_open(struct pstore_info *psi)
 	if (err)
 		return err;
 
-	psi->data = kzalloc(EFIVARS_DATA_SIZE_MAX, GFP_KERNEL);
+	psi->data = kzalloc(record_size, GFP_KERNEL);
 	if (!psi->data)
 		return -ENOMEM;
 
@@ -52,7 +54,7 @@ static inline u64 generic_id(u64 timestamp, unsigned int part, int count)
 static int efi_pstore_read_func(struct pstore_record *record,
 				efi_char16_t *varname)
 {
-	unsigned long wlen, size = EFIVARS_DATA_SIZE_MAX;
+	unsigned long wlen, size = record_size;
 	char name[DUMP_NAME_LEN], data_type;
 	efi_status_t status;
 	int cnt;
@@ -133,7 +135,7 @@ static ssize_t efi_pstore_read(struct pstore_record *record)
 	efi_status_t status;
 
 	for (;;) {
-		varname_size = EFIVARS_DATA_SIZE_MAX;
+		varname_size = record_size;
 
 		/*
 		 * If this is the first read() call in the pstore enumeration,
@@ -224,11 +226,14 @@ static __init int efivars_pstore_init(void)
 	if (efivars_pstore_disable)
 		return 0;
 
+	if (record_size < 1024)
+		record_size = 1024;
+
 	efi_pstore_info.buf = kmalloc(4096, GFP_KERNEL);
 	if (!efi_pstore_info.buf)
 		return -ENOMEM;
 
-	efi_pstore_info.bufsize = 1024;
+	efi_pstore_info.bufsize = record_size;
 
 	if (pstore_register(&efi_pstore_info)) {
 		kfree(efi_pstore_info.buf);
-- 
2.38.0

Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Kees Cook]

On Thu, Oct 06, 2022 at 07:42:12PM -0300, Guilherme G. Piccoli wrote:
> By default, the efi-pstore backend hardcode the UEFI variable size
> as 1024 bytes. That's not a big deal, but at the same time having
> no way to change that in the kernel is a bit bummer for specialized
> users - there is not such a limit in the UEFI specification.

It seems to have been added in commit
e0d59733f6b1 ("efivars, efi-pstore: Hold off deletion of sysfs entry until the scan is completed")

But I see no mention of why it was introduced or how it was chosen.

I remember hearing some horror stories from Matthew Garrett about older
EFI implementations bricking themselves when they stored large
variables, or something like that, but I don't know if that's meaningful
here at all.

I think it'd be great to make it configurable! Ard, do you have any
sense of what the max/min, etc, should be here?

-- 
Kees Cook

Re: [PATCH 7/8] efi: pstore: Follow convention for the efi-pstore backend name

[Kees Cook]

On Thu, Oct 06, 2022 at 07:42:11PM -0300, Guilherme G. Piccoli wrote:
> For some reason, the efi-pstore backend name (exposed through the
> pstore infrastructure) is hardcoded as "efi", whereas all the other
> backends follow a kind of convention in using the module name.
> 
> Let's do it here as well, to make user's life easier (they might
> use this info for unloading the module backend, for example).
> 
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>

Looks fine to me. Ard, if you don't object, I can carry this in the
pstore tree.

-- 
Kees Cook

Re: [PATCH 0/8] Some pstore improvements

[Kees Cook]

On Thu, 6 Oct 2022 19:42:04 -0300, Guilherme G. Piccoli wrote:
> overall. Most of them are minors, but the implicit conversion thing
> is a bit more "relevant" in a sense it's more invasive and would fit
> more as a "fix".
> 
> The code is based on v6.0, and it was tested with multiple compression
> algorithms (zstd, deflate, lz4, lzo, 842) and two backends (ramoops and
> efi_pstore) - I've used a QEMU UEFI guest and Steam Deck for this goal.
> 
> [...]

Applied to for-next/pstore, thanks!

[1/8] pstore: Improve error reporting in case of backend overlap
      https://git.kernel.org/kees/c/55dbe25ee4c8
[2/8] pstore: Expose kmsg_bytes as a module parameter
      https://git.kernel.org/kees/c/1af13c2b6324
[3/8] pstore: Inform unregistered backend names as well
      https://git.kernel.org/kees/c/a4f92789f799

-- 
Kees Cook

Re: [PATCH 7/8] efi: pstore: Follow convention for the efi-pstore backend name

[Ard Biesheuvel]

On Fri, 7 Oct 2022 at 01:16, Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Oct 06, 2022 at 07:42:11PM -0300, Guilherme G. Piccoli wrote:
> > For some reason, the efi-pstore backend name (exposed through the
> > pstore infrastructure) is hardcoded as "efi", whereas all the other
> > backends follow a kind of convention in using the module name.
> >
> > Let's do it here as well, to make user's life easier (they might
> > use this info for unloading the module backend, for example).
> >
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
>
> Looks fine to me. Ard, if you don't object, I can carry this in the
> pstore tree.
>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Ard Biesheuvel]

On Fri, 7 Oct 2022 at 01:16, Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Oct 06, 2022 at 07:42:12PM -0300, Guilherme G. Piccoli wrote:
> > By default, the efi-pstore backend hardcode the UEFI variable size
> > as 1024 bytes. That's not a big deal, but at the same time having
> > no way to change that in the kernel is a bit bummer for specialized
> > users - there is not such a limit in the UEFI specification.
>
> It seems to have been added in commit
> e0d59733f6b1 ("efivars, efi-pstore: Hold off deletion of sysfs entry until the scan is completed")
>

Yeah fortunately that kludge is gone now.

> But I see no mention of why it was introduced or how it was chosen.
>

There is some cargo cult from prehistoric EFI times going on here, it
seems. Or maybe just misinterpretation of the maximum size for the
variable *name* vs the variable itself.

> I remember hearing some horror stories from Matthew Garrett about older
> EFI implementations bricking themselves when they stored large
> variables, or something like that, but I don't know if that's meaningful
> here at all.
>

This was related to filling up the variable store to the point where
SetVariable() calls in the firmware itself would start failing,
bricking the system in the process.

The efivars layer below efi-pstore will take care of this, by ensuring
upfront that sufficient space is available.

> I think it'd be great to make it configurable! Ard, do you have any
> sense of what the max/min, etc, should be here?
>

Given that dbx on an arbitrary EFI system with secure boot enabled is
already almost 4k, that seems like a reasonable default. As for the
upper bound, there is no way to know what weird firmware bugs you
might tickle by choosing highly unusual values here.

If you need to store lots of data, you might want to look at [0] for
some work done in the past on using capsule update for preserving data
across a reboot. In the general case, this is not as useful, as the
capsule is only delivered to the firmware after invoking the
ResetSystem() EFI runtime service (as opposed to SetVariable() calls
taking effect immediately). However, if you need to capture large
amounts of data, and can tolerate the uncertainty involved in the
capsule approach, it might be a reasonable option.

[0] https://lore.kernel.org/all/20200312011335.70750-1-qiuxu.zhuo@intel.com/

Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Guilherme G. Piccoli]

First of all, thanks Ard for the historical explanation!


On 07/10/2022 06:11, Ard Biesheuvel wrote:
> [...]
>> I think it'd be great to make it configurable! Ard, do you have any
>> sense of what the max/min, etc, should be here?
>>
> 
> Given that dbx on an arbitrary EFI system with secure boot enabled is
> already almost 4k, that seems like a reasonable default. As for the
> upper bound, there is no way to know what weird firmware bugs you
> might tickle by choosing highly unusual values here.
> 
> If you need to store lots of data, you might want to look at [0] for
> some work done in the past on using capsule update for preserving data
> across a reboot. In the general case, this is not as useful, as the
> capsule is only delivered to the firmware after invoking the
> ResetSystem() EFI runtime service (as opposed to SetVariable() calls
> taking effect immediately). However, if you need to capture large
> amounts of data, and can tolerate the uncertainty involved in the
> capsule approach, it might be a reasonable option.
> 
> [0] https://lore.kernel.org/all/20200312011335.70750-1-qiuxu.zhuo@intel.com/

So, you mean 4K as the default? I can change, but I would try to not
mess with the current users, is there a case you can imagine something
like 4k would fail? Maybe 2K is safer?

As for the maximum, I've tested with many values, and when it's larger
than ~30000 for edk2/ovmf, it fails with EFI_OUT_OF_RESOURCES and
doesn't collect the log; other than that, no issues observed.

When set to ~24000, the interesting is that we have fewer big logs in
/sys/fs/pstore, so it's nice to see compared to the bunch of 1K files heheh

Anyway, let's agree on the default and then I can resubmit that, I'm
glad you both consider that it's a good idea =)

Thanks,


Guilherme

Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Ard Biesheuvel]

On Fri, 7 Oct 2022 at 15:00, Guilherme G. Piccoli <gpiccoli@igalia.com> wrote:
>
> First of all, thanks Ard for the historical explanation!
>
>
> On 07/10/2022 06:11, Ard Biesheuvel wrote:
> > [...]
> >> I think it'd be great to make it configurable! Ard, do you have any
> >> sense of what the max/min, etc, should be here?
> >>
> >
> > Given that dbx on an arbitrary EFI system with secure boot enabled is
> > already almost 4k, that seems like a reasonable default. As for the
> > upper bound, there is no way to know what weird firmware bugs you
> > might tickle by choosing highly unusual values here.
> >
> > If you need to store lots of data, you might want to look at [0] for
> > some work done in the past on using capsule update for preserving data
> > across a reboot. In the general case, this is not as useful, as the
> > capsule is only delivered to the firmware after invoking the
> > ResetSystem() EFI runtime service (as opposed to SetVariable() calls
> > taking effect immediately). However, if you need to capture large
> > amounts of data, and can tolerate the uncertainty involved in the
> > capsule approach, it might be a reasonable option.
> >
> > [0] https://lore.kernel.org/all/20200312011335.70750-1-qiuxu.zhuo@intel.com/
>
> So, you mean 4K as the default? I can change, but I would try to not
> mess with the current users, is there a case you can imagine something
> like 4k would fail? Maybe 2K is safer?
>

Reducing the number of writes 4x on systems that can support this has
its own advantages, so I am willing to risk it.

> As for the maximum, I've tested with many values, and when it's larger
> than ~30000 for edk2/ovmf, it fails with EFI_OUT_OF_RESOURCES and
> doesn't collect the log; other than that, no issues observed.
>

OVMF has

OvmfPkg/OvmfPkgX64.dsc:
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
OvmfPkg/OvmfPkgX64.dsc:
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400

where the first one is without secure boot and the second with secure boot.

Interestingly, the default is

gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400

so this is probably where this 1k number comes from. So perhaps it is
better to leave it at 1k after all :-(

> When set to ~24000, the interesting is that we have fewer big logs in
> /sys/fs/pstore, so it's nice to see compared to the bunch of 1K files heheh
>
> Anyway, let's agree on the default and then I can resubmit that, I'm
> glad you both consider that it's a good idea =)
>
> Thanks,
>
>
> Guilherme

Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Guilherme G. Piccoli]

On 07/10/2022 10:19, Ard Biesheuvel wrote:
> [...]
> 
> OVMF has
> 
> OvmfPkg/OvmfPkgX64.dsc:
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> OvmfPkg/OvmfPkgX64.dsc:
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> 
> where the first one is without secure boot and the second with secure boot.
> 
> Interestingly, the default is
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400
> 
> so this is probably where this 1k number comes from. So perhaps it is
> better to leave it at 1k after all :-(
> 

Oh darn...

So, let's stick with 1024 then? If so, no need for re-submitting right?
Thanks again,


Guilherme

Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Ard Biesheuvel]

On Fri, 7 Oct 2022 at 15:46, Guilherme G. Piccoli <gpiccoli@igalia.com> wrote:
>
> On 07/10/2022 10:19, Ard Biesheuvel wrote:
> > [...]
> >
> > OVMF has
> >
> > OvmfPkg/OvmfPkgX64.dsc:
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> > OvmfPkg/OvmfPkgX64.dsc:
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> >
> > where the first one is without secure boot and the second with secure boot.
> >
> > Interestingly, the default is
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400
> >
> > so this is probably where this 1k number comes from. So perhaps it is
> > better to leave it at 1k after all :-(
> >
>
> Oh darn...
>
> So, let's stick with 1024 then? If so, no need for re-submitting right?

Well, I did spot this oddity

        efi_pstore_info.buf = kmalloc(4096, GFP_KERNEL);
        if (!efi_pstore_info.buf)
                return -ENOMEM;

       efi_pstore_info.bufsize = 1024;

So that hardcoded 4096 looks odd, but at least it is larger than the
default 1024. So what happens if you increase the record size to >
4096?

Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Guilherme G. Piccoli]

On 07/10/2022 12:06, Ard Biesheuvel wrote:
> [...]
> Well, I did spot this oddity
> 
>         efi_pstore_info.buf = kmalloc(4096, GFP_KERNEL);
>         if (!efi_pstore_info.buf)
>                 return -ENOMEM;
> 
>        efi_pstore_info.bufsize = 1024;
> 
> So that hardcoded 4096 looks odd, but at least it is larger than the
> default 1024. So what happens if you increase the record size to >
> 4096?

This is a very good finding, thanks a bunch Ard and apologies for this
mistake!

Before this patch it was "safe" doing this way since the allocation was
4096 whereas the size value was 1024. Now, with my change this is not
valid anymore, and my feeling is that it worked fine in my tests because
I'm testing panic (which is a single CPU/no-IRQ scenario), so basically
we're corrupting memory...but nothing broke in my tests due to panic
scenario.

Thanks again, I'll fix that - need to allocate record_size.


Guilherme

Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Kees Cook]

On Fri, Oct 07, 2022 at 10:45:33AM -0300, Guilherme G. Piccoli wrote:
> On 07/10/2022 10:19, Ard Biesheuvel wrote:
> > [...]
> > 
> > OVMF has
> > 
> > OvmfPkg/OvmfPkgX64.dsc:
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> > OvmfPkg/OvmfPkgX64.dsc:
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> > 
> > where the first one is without secure boot and the second with secure boot.
> > 
> > Interestingly, the default is
> > 
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400
> > 
> > so this is probably where this 1k number comes from. So perhaps it is
> > better to leave it at 1k after all :-(
> > 
> 
> Oh darn...
> 
> So, let's stick with 1024 then? If so, no need for re-submitting right?

Given OVMF showing this as a max, it doesn't seem right to also make
this a minimum? Perhaps choose a different minimum to be enforced.

Also, can you update the commit log with Ard's archeology on
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ?

-- 
Kees Cook

Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Guilherme G. Piccoli]

On 07/10/2022 16:32, Kees Cook wrote:
> [...]
> Given OVMF showing this as a max, it doesn't seem right to also make
> this a minimum? Perhaps choose a different minimum to be enforced.

Hi Kees! Through my tests, I've noticed low values tend to cause issues
(didn't go further in the investigation), IIRC even 512 caused problems
on "deflate" (worked in the others).

I'll try again 512 to see how it goes, but I'm not so sure what would be
the use of such low values, it does truncate a lot and "pollute" the
pstore fs with many small files. But I can go with any value you/Ard
think is appropriate (given it works with all compression algorithms
heh) - currently the minimum of 1024 is enforced in the patch.

> 
> Also, can you update the commit log with Ard's archeology on
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ?
> 

Sure, of course!
Cheers,


Guilherme

Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

[Kees Cook]

On October 7, 2022 4:29:55 PM PDT, "Guilherme G. Piccoli" <gpiccoli@igalia.com> wrote:
>On 07/10/2022 16:32, Kees Cook wrote:
>> [...]
>> Given OVMF showing this as a max, it doesn't seem right to also make
>> this a minimum? Perhaps choose a different minimum to be enforced.
>
>Hi Kees! Through my tests, I've noticed low values tend to cause issues
>(didn't go further in the investigation), IIRC even 512 caused problems
>on "deflate" (worked in the others).
>
>I'll try again 512 to see how it goes, but I'm not so sure what would be
>the use of such low values, it does truncate a lot and "pollute" the
>pstore fs with many small files. But I can go with any value you/Ard
>think is appropriate (given it works with all compression algorithms
>heh) - currently the minimum of 1024 is enforced in the patch.

Right, but not everyone uses compression. On the other hand, this was never configurable before, so, sure, let's do 1k as a minimum. (And a comment in the source.)


-- 
Kees Cook

Re: [PATCH 0/8] Some pstore improvements

[Guilherme G. Piccoli]

On 06/10/2022 20:24, Kees Cook wrote:
> On Thu, 6 Oct 2022 19:42:04 -0300, Guilherme G. Piccoli wrote:
>> overall. Most of them are minors, but the implicit conversion thing
>> is a bit more "relevant" in a sense it's more invasive and would fit
>> more as a "fix".
>>
>> The code is based on v6.0, and it was tested with multiple compression
>> algorithms (zstd, deflate, lz4, lzo, 842) and two backends (ramoops and
>> efi_pstore) - I've used a QEMU UEFI guest and Steam Deck for this goal.
>>
>> [...]
> 
> Applied to for-next/pstore, thanks!
> 
> [1/8] pstore: Improve error reporting in case of backend overlap
>       https://git.kernel.org/kees/c/55dbe25ee4c8
> [2/8] pstore: Expose kmsg_bytes as a module parameter
>       https://git.kernel.org/kees/c/1af13c2b6324
> [3/8] pstore: Inform unregistered backend names as well
>       https://git.kernel.org/kees/c/a4f92789f799
> 

Thanks Kees! just a heads-up on how I'll proceed.

(a) Patches 1-3 were added already.

(b) MAINTAINERS patch was reworked by yourself in the other series, so
I'll discard my version.

(c) I'll rework patches 4 and 8 and re-submit them plus patch 7
(including the ACK from Ard).

(d) Gonna discard for now patch 5, planning to test a new version on top
of the crypto acomp interface V2 from Ard/you.

Cheers,


Guilherme

Re: [PATCH 0/8] Some pstore improvements

[Kees Cook]

On Wed, Oct 12, 2022 at 12:50:50PM -0300, Guilherme G. Piccoli wrote:
> On 06/10/2022 20:24, Kees Cook wrote:
> > On Thu, 6 Oct 2022 19:42:04 -0300, Guilherme G. Piccoli wrote:
> >> overall. Most of them are minors, but the implicit conversion thing
> >> is a bit more "relevant" in a sense it's more invasive and would fit
> >> more as a "fix".
> >>
> >> The code is based on v6.0, and it was tested with multiple compression
> >> algorithms (zstd, deflate, lz4, lzo, 842) and two backends (ramoops and
> >> efi_pstore) - I've used a QEMU UEFI guest and Steam Deck for this goal.
> >>
> >> [...]
> > 
> > Applied to for-next/pstore, thanks!
> > 
> > [1/8] pstore: Improve error reporting in case of backend overlap
> >       https://git.kernel.org/kees/c/55dbe25ee4c8
> > [2/8] pstore: Expose kmsg_bytes as a module parameter
> >       https://git.kernel.org/kees/c/1af13c2b6324
> > [3/8] pstore: Inform unregistered backend names as well
> >       https://git.kernel.org/kees/c/a4f92789f799
> > 
> 
> Thanks Kees! just a heads-up on how I'll proceed.
> 
> (a) Patches 1-3 were added already.
> 
> (b) MAINTAINERS patch was reworked by yourself in the other series, so
> I'll discard my version.
> 
> (c) I'll rework patches 4 and 8 and re-submit them plus patch 7
> (including the ACK from Ard).
> 
> (d) Gonna discard for now patch 5, planning to test a new version on top
> of the crypto acomp interface V2 from Ard/you.

Sounds good; thanks!

-- 
Kees Cook

Re: (subset) [PATCH 7/8] efi: pstore: Follow convention for the efi-pstore backend name

[Kees Cook]

On Thu, 6 Oct 2022 19:42:11 -0300, Guilherme G. Piccoli wrote:
> For some reason, the efi-pstore backend name (exposed through the
> pstore infrastructure) is hardcoded as "efi", whereas all the other
> backends follow a kind of convention in using the module name.
> 
> Let's do it here as well, to make user's life easier (they might
> use this info for unloading the module backend, for example).
> 
> [...]

Applied to for-next/pstore, thanks!

[7/8] efi: pstore: Follow convention for the efi-pstore backend name
      https://git.kernel.org/kees/c/39bae0ee0656

-- 
Kees Cook