Re: [PATCH v21 2/7] crash: add generic infrastructure for crash hotplug support
On 04/18/23 at 08:55am, Eric DeVolder wrote: .. > > > > Seems we passed in the cpu number just for printing here. Wondering why > > > > we don't print out hot added/removed memory ranges. Is the cpu number > > > > printing necessary? > > > > > > > Baoquan, > > > > > > Ah, actually until recently it was used to track the 'offlinecpu' in this > > > function, but tglx pointed out that was un-necessary. That resulted in > > > dropping the code in this function dealing with offlinecpu, leaving this > > > as > > > its only use in this function. > > > > > > The printing of cpu number is not necessary, but helpful; I use it for > > > debugging. > > > > OK, I see. I am not requesting memory range printing, just try to prove > > cpu number printing is not so justified. If it's helpful, I am OK with > > it. Let's see if other people have concern about this. > > > > I do not plan on adding the memory range printing. > > > > > > > The printing of memory range is also not necessary, but in order to do > > > that, > > > should we choose to do so, requires passing in the memory range to this > > > function. This patch series did do this early on, and by v7 I dropped it > > > at > > > your urging > > > (https://lore.kernel.org/lkml/20220401183040.1624-1-eric.devol...@oracle.com/). > > > At the time, I provided it since I considered this generic infrastructure, > > > but I could not defend it since x86 didn't need it. However, PPC now needs > > > this, and is now carrying this as part of PPC support of CRASH_HOTPLUG > > > (https://lore.kernel.org/linuxppc-dev/20230312181154.278900-6-sourabhj...@linux.ibm.com/T/#u). > > > > > > If you'd rather I pickup the memory range handling again, I can do that. I > > > think I'd likely change this function to be: > > > > > >void crash_handle_hotplug_event(unsigned int hp_action, unsigned int > > > cpu, > > > struct memory_notify *mhp); > > > > > > where on a CPU op the 'cpu' parameter would be valid and 'mhp' NULL, and > > > on a memory op, > > > the 'mhp' would be valid and 'cpu' parameter invalid(0). > > > > > > I'd likely then stuff these two parameters into struct kimage so that it > > > can > > > be utilized by arch-specific handler, if needed. > > > > > > And of course, would print out the memory range for debug purposes. > > > > > > Let me know what you think. > > > > I do not plan on adding the memory range handling; I'll let Sourabh do that > as he has a use case for it. > > As such, I don't see any other request for changes. OK, then I have no concern about this patchset. Thanks a lot for all these effort, Eric. Hi x86 maintainers, Could you help check if there's anything we need improve, or consider taking this patchset? Thanks Baoquan ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v9 4/4] tpm/kexec: Duplicate TPM measurement log in of-tree for kexec
Hi Stefan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 6a8f57ae2eb07ab39a6f0ccad60c760743051026]
url:
https://github.com/intel-lab-lkp/linux/commits/Stefan-Berger/drivers-of-kexec-ima-Support-32-bit-platforms/20230418-214600
base: 6a8f57ae2eb07ab39a6f0ccad60c760743051026
patch link:
https://lore.kernel.org/r/20230418134409.177485-5-stefanb%40linux.ibm.com
patch subject: [PATCH v9 4/4] tpm/kexec: Duplicate TPM measurement log in
of-tree for kexec
config: x86_64-randconfig-a013-20230417
(https://download.01.org/0day-ci/archive/20230419/202304190215.d0zwo1ni-...@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project
f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/16a833d47b9aca53a1b099dea4066b76b7f14ee1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Stefan-Berger/drivers-of-kexec-ima-Support-32-bit-platforms/20230418-214600
git checkout 16a833d47b9aca53a1b099dea4066b76b7f14ee1
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/mailbox/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot
| Link:
https://lore.kernel.org/oe-kbuild-all/202304190215.d0zwo1ni-...@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/mailbox/mailbox.c:18:
In file included from include/linux/mailbox_client.h:10:
>> include/linux/of.h:1664:48: warning: declaration of 'struct kimage' will not
>> be visible outside of this function [-Wvisibility]
static inline void tpm_add_kexec_buffer(struct kimage *image) { }
^
1 warning generated.
vim +1664 include/linux/of.h
1660
1661 #if defined(CONFIG_KEXEC_FILE) && defined(CONFIG_OF_FLATTREE)
1662 void tpm_add_kexec_buffer(struct kimage *image);
1663 #else
> 1664 static inline void tpm_add_kexec_buffer(struct kimage *image) { }
1665 #endif
1666
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v9 4/4] tpm/kexec: Duplicate TPM measurement log in of-tree for kexec
On Tue, Apr 18, 2023 at 09:44:09AM -0400, Stefan Berger wrote:
> The memory area of the TPM measurement log is currently not properly
> duplicated for carrying it across kexec when an Open Firmware
> Devicetree is used. Therefore, the contents of the log get corrupted.
> Fix this for the kexec_file_load() syscall by allocating a buffer and
> copying the contents of the existing log into it. The new buffer is
> preserved across the kexec and a pointer to it is available when the new
> kernel is started. To achieve this, store the allocated buffer's address
> in the flattened device tree (fdt) under the name linux,tpm-kexec-buffer
> and search for this entry early in the kernel startup before the TPM
> subsystem starts up. Adjust the pointer in the of-tree stored under
> linux,sml-base to point to this buffer holding the preserved log. The TPM
> driver can then read the base address from this entry when making the log
> available. Invalidate the log by removing 'linux,sml-base' from the
> devicetree if anything goes wrong with updating the buffer.
>
> Use subsys_initcall() to call the function to restore the buffer even if
> the TPM subsystem or driver are not used. This allows the buffer to be
> carried across the next kexec without involvement of the TPM subsystem
> and ensures a valid buffer pointed to by the of-tree.
Hi Stefan,
some minor feedback from my side.
> Use the subsys_initcall(), rather than an ealier initcall, since
nit via checkpatch.pl --codespell: s/ealier/earlier/
> page_is_ram() in get_kexec_buffer() only starts working at this stage.
>
> Signed-off-by: Stefan Berger
> Cc: Rob Herring
> Cc: Frank Rowand
> Cc: Eric Biederman
> Tested-by: Nageswara R Sastry
> Tested-by: Coiby Xu
> Reviewed-by: Rob Herring
...
> +void tpm_add_kexec_buffer(struct kimage *image)
> +{
> + struct kexec_buf kbuf = { .image = image, .buf_align = 1,
> + .buf_min = 0, .buf_max = ULONG_MAX,
> + .top_down = true };
> + struct device_node *np;
> + void *buffer;
> + u32 size;
> + u64 base;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_PPC64))
> + return;
> +
> + np = of_find_node_by_name(NULL, "vtpm");
> + if (!np)
> + return;
> +
> + if (of_tpm_get_sml_parameters(np, , ) < 0)
> + return;
> +
> + buffer = vmalloc(size);
> + if (!buffer)
> + return;
> + memcpy(buffer, __va(base), size);
> +
> + kbuf.buffer = buffer;
> + kbuf.bufsz = size;
> + kbuf.memsz = size;
> + ret = kexec_add_buffer();
> + if (ret) {
> + pr_err("Error passing over kexec TPM measurement log buffer:
> %d\n",
> +ret);
Does buffer need to be freed here?
> + return;
> + }
> +
> + image->tpm_buffer = buffer;
> + image->tpm_buffer_addr = kbuf.mem;
> + image->tpm_buffer_size = size;
> +}
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v9 4/4] tpm/kexec: Duplicate TPM measurement log in of-tree for kexec
Hi Stefan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 6a8f57ae2eb07ab39a6f0ccad60c760743051026]
url:
https://github.com/intel-lab-lkp/linux/commits/Stefan-Berger/drivers-of-kexec-ima-Support-32-bit-platforms/20230418-214600
base: 6a8f57ae2eb07ab39a6f0ccad60c760743051026
patch link:
https://lore.kernel.org/r/20230418134409.177485-5-stefanb%40linux.ibm.com
patch subject: [PATCH v9 4/4] tpm/kexec: Duplicate TPM measurement log in
of-tree for kexec
config: um-i386_defconfig
(https://download.01.org/0day-ci/archive/20230419/202304190146.frlhobob-...@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
#
https://github.com/intel-lab-lkp/linux/commit/16a833d47b9aca53a1b099dea4066b76b7f14ee1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Stefan-Berger/drivers-of-kexec-ima-Support-32-bit-platforms/20230418-214600
git checkout 16a833d47b9aca53a1b099dea4066b76b7f14ee1
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=um SUBARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash
drivers/input/mouse/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot
| Link:
https://lore.kernel.org/oe-kbuild-all/202304190146.frlhobob-...@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/irqdomain.h:36,
from include/linux/acpi.h:13,
from include/linux/i2c.h:13,
from drivers/input/mouse/synaptics.c:30:
>> include/linux/of.h:1664:48: warning: 'struct kimage' declared inside
>> parameter list will not be visible outside of this definition or declaration
1664 | static inline void tpm_add_kexec_buffer(struct kimage *image) { }
|^~
drivers/input/mouse/synaptics.c:164:27: warning: 'smbus_pnp_ids' defined but
not used [-Wunused-const-variable=]
164 | static const char * const smbus_pnp_ids[] = {
| ^
vim +1664 include/linux/of.h
1660
1661 #if defined(CONFIG_KEXEC_FILE) && defined(CONFIG_OF_FLATTREE)
1662 void tpm_add_kexec_buffer(struct kimage *image);
1663 #else
> 1664 static inline void tpm_add_kexec_buffer(struct kimage *image) { }
1665 #endif
1666
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v5 2/2] x86/purgatory: Add linker script
On Tue, Apr 11, 2023 at 2:46 PM Ricardo Ribalda wrote:
>
> Hi Nick
>
> On Sat, 8 Apr 2023 at 01:22, Nick Desaulniers wrote:
> >
> > Hi Ricardo,
> > Thanks for the patch! Please make sure to cc our mailing list
> > for llvm specific issues.
> > scripts/get_maintainer.pl should recommend it, or you can find it from
> > clangbuiltlinux.github.io. You can also ping me internally for
> > toolchain related issues.
> >
> > Start of thread.
> > https://lore.kernel.org/lkml/20230321-kexec_clang16-v5-0-5563bf7c4...@chromium.org/
> >
> > On Thu, Mar 30, 2023 at 9:00 AM Borislav Petkov wrote:
> > >
> > > On Thu, Mar 30, 2023 at 11:31:27AM -0400, Steven Rostedt wrote:
> > > > On Thu, 30 Mar 2023 17:18:26 +0200
> > > > Borislav Petkov wrote:
> > > >
> > > > > On Thu, Mar 30, 2023 at 11:15:23AM -0400, Steven Rostedt wrote:
> > > > > > > Make sure that the .text section is not divided in multiple
> > > > > > > overlapping
> > > > > > > sections. This is not supported by kexec_file.
> >
> > Perhaps this is related to CrOS' use of AutoFDO creating .text.hot?
> > If so, it's probably more straightforward to straight up disable PGO
> > for kexec. See also:
> >
> > commit bde971a83bbf ("KVM: arm64: nvhe: Fix build with profile
> > optimization")
>
> It was indeed due to the AutoFDO, adding
>
> KBUILD_CFLAGS := $(filter-out -fprofile-sample-use=% -fprofile-use=%,
> $(KBUILD_CFLAGS))
>
> to arch/x86/purgatory/Makefile
>
> It is definitely simpler than adding a linker script, but I am not
> sure if it is the correct way to fix this... Seems like splitting
> .text in multiple sections is an implementation detail of the compiler
> and the only way to force it is with a linker script... Or am I
> missing something?
I think with the use of `unlikely` GCC will put code in .text.cold, so
it is possible to trigger this using simpler means, but...
>
> Shall I send a new version with the KBUILD_CFLAGS ?
I still think the cflags approach is way simpler. If someone tries to
use unlikely in purgatory: "don't do that." Same for PGO.
>
> Thanks!
>
> >
> > > > >
> > > > > And?
> > > > >
> > > > > What is the failure scenario? Why are you fixing it? Why do we care?
> > > > >
> > > > > This is way too laconic.
> > > > >
> > > >
> > > > Yeah, I think the change log in patch 1 needs to be in this patch too,
> > > > which gives better context.
> > >
> > > Just read it.
> > >
> > > Why did it work with clang version < 16?
> >
> > I'll bet if we bisect llvm, we can spot what might have changed, which
> > may give us a clue on how to get the old behavior back; maybe without
> > the need for a linker script.
> >
> > Ricardo, how did you verify that your fix was correct? Surely we can
> > check using command line utilities without needing a full blown kexec
> > setup? If you can share more info, I can bisect llvm quickly. If it
> > requires profile data, you'll need to share it, since CrOS engineers
> > still have not posted public documentation on AutoFDO as I have
> > repeatedly asked for.
>
> The simplest test is to run:
>
> $readelf -S arch/x86/purgatory/purgatory.ro | grep "] \.text"
> [ 3] .text PROGBITS 02a0
>
> If there is only one .text section then that kernel will be load
> properly via kexec_file().
Got it, profile data will be required to reproduce then. If you can share.
--
Thanks,
~Nick Desaulniers
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v21 2/7] crash: add generic infrastructure for crash hotplug support
On 4/6/23 18:58, Baoquan He wrote:
On 04/06/23 at 11:10am, Eric DeVolder wrote:
On 4/6/23 06:04, Baoquan He wrote:
On 04/04/23 at 02:03pm, Eric DeVolder wrote:
..
+static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int
cpu)
+{
+ struct kimage *image;
+
+ /* Obtain lock while changing crash information */
+ if (!kexec_trylock()) {
+ pr_info("kexec_trylock() failed, elfcorehdr may be
inaccurate\n");
+ return;
+ }
+
+ /* Check kdump is not loaded */
+ if (!kexec_crash_image)
+ goto out;
+
+ image = kexec_crash_image;
+
+ if (hp_action == KEXEC_CRASH_HP_ADD_CPU ||
+ hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
+ pr_debug("hp_action %u, cpu %u\n", hp_action, cpu);
+ else
+ pr_debug("hp_action %u\n", hp_action);
Seems we passed in the cpu number just for printing here. Wondering why
we don't print out hot added/removed memory ranges. Is the cpu number
printing necessary?
Baoquan,
Ah, actually until recently it was used to track the 'offlinecpu' in this
function, but tglx pointed out that was un-necessary. That resulted in
dropping the code in this function dealing with offlinecpu, leaving this as
its only use in this function.
The printing of cpu number is not necessary, but helpful; I use it for
debugging.
OK, I see. I am not requesting memory range printing, just try to prove
cpu number printing is not so justified. If it's helpful, I am OK with
it. Let's see if other people have concern about this.
I do not plan on adding the memory range printing.
The printing of memory range is also not necessary, but in order to do that,
should we choose to do so, requires passing in the memory range to this
function. This patch series did do this early on, and by v7 I dropped it at
your urging
(https://lore.kernel.org/lkml/20220401183040.1624-1-eric.devol...@oracle.com/).
At the time, I provided it since I considered this generic infrastructure,
but I could not defend it since x86 didn't need it. However, PPC now needs
this, and is now carrying this as part of PPC support of CRASH_HOTPLUG
(https://lore.kernel.org/linuxppc-dev/20230312181154.278900-6-sourabhj...@linux.ibm.com/T/#u).
If you'd rather I pickup the memory range handling again, I can do that. I
think I'd likely change this function to be:
void crash_handle_hotplug_event(unsigned int hp_action, unsigned int cpu,
struct memory_notify *mhp);
where on a CPU op the 'cpu' parameter would be valid and 'mhp' NULL, and on a
memory op,
the 'mhp' would be valid and 'cpu' parameter invalid(0).
I'd likely then stuff these two parameters into struct kimage so that it can
be utilized by arch-specific handler, if needed.
And of course, would print out the memory range for debug purposes.
Let me know what you think.
I do not plan on adding the memory range handling; I'll let Sourabh do that as
he has a use case for it.
As such, I don't see any other request for changes.
Thanks!
eric
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v9 4/4] tpm/kexec: Duplicate TPM measurement log in of-tree for kexec
The memory area of the TPM measurement log is currently not properly
duplicated for carrying it across kexec when an Open Firmware
Devicetree is used. Therefore, the contents of the log get corrupted.
Fix this for the kexec_file_load() syscall by allocating a buffer and
copying the contents of the existing log into it. The new buffer is
preserved across the kexec and a pointer to it is available when the new
kernel is started. To achieve this, store the allocated buffer's address
in the flattened device tree (fdt) under the name linux,tpm-kexec-buffer
and search for this entry early in the kernel startup before the TPM
subsystem starts up. Adjust the pointer in the of-tree stored under
linux,sml-base to point to this buffer holding the preserved log. The TPM
driver can then read the base address from this entry when making the log
available. Invalidate the log by removing 'linux,sml-base' from the
devicetree if anything goes wrong with updating the buffer.
Use subsys_initcall() to call the function to restore the buffer even if
the TPM subsystem or driver are not used. This allows the buffer to be
carried across the next kexec without involvement of the TPM subsystem
and ensures a valid buffer pointed to by the of-tree.
Use the subsys_initcall(), rather than an ealier initcall, since
page_is_ram() in get_kexec_buffer() only starts working at this stage.
Signed-off-by: Stefan Berger
Cc: Rob Herring
Cc: Frank Rowand
Cc: Eric Biederman
Tested-by: Nageswara R Sastry
Tested-by: Coiby Xu
Reviewed-by: Rob Herring
---
v6:
- Define prototype for tpm_add_kexec_buffer under same config options
as drivers/of/kexec.c is compiled, provide inline function otherwise.
(kernel test robot)
v4:
- Added #include due to parisc
- Use phys_addr_t for physical address rather than void *
- Remove linux,sml-base if the buffer cannot be updated after a kexec
- Added __init to functions where possible
---
drivers/of/kexec.c| 216 +-
include/linux/kexec.h | 6 ++
include/linux/of.h| 6 ++
kernel/kexec_file.c | 6 ++
4 files changed, 232 insertions(+), 2 deletions(-)
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index fa8c0c75adf9..9831d25dd83e 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -19,6 +19,8 @@
#include
#include
#include
+#include
+#include
#define RNG_SEED_SIZE 128
@@ -116,7 +118,6 @@ static int do_get_kexec_buffer(const void *prop, int len,
unsigned long *addr,
return 0;
}
-#ifdef CONFIG_HAVE_IMA_KEXEC
static int __init get_kexec_buffer(const char *name, unsigned long *addr,
size_t *size)
{
@@ -151,6 +152,7 @@ static int __init get_kexec_buffer(const char *name,
unsigned long *addr,
return 0;
}
+#ifdef CONFIG_HAVE_IMA_KEXEC
/**
* ima_get_kexec_buffer - get IMA buffer from the previous kernel
* @addr: On successful return, set to point to the buffer contents.
@@ -239,7 +241,6 @@ static void remove_ima_buffer(void *fdt, int chosen_node)
remove_buffer(fdt, chosen_node, "linux,ima-kexec-buffer");
}
-#ifdef CONFIG_IMA_KEXEC
static int setup_buffer(void *fdt, int chosen_node, const char *name,
phys_addr_t addr, size_t size)
{
@@ -263,6 +264,7 @@ static int setup_buffer(void *fdt, int chosen_node, const
char *name,
}
+#ifdef CONFIG_IMA_KEXEC
/**
* setup_ima_buffer - add IMA buffer information to the fdt
* @image: kexec image being loaded.
@@ -285,6 +287,213 @@ static inline int setup_ima_buffer(const struct kimage
*image, void *fdt,
}
#endif /* CONFIG_IMA_KEXEC */
+/**
+ * tpm_get_kexec_buffer - get TPM log buffer from the previous kernel
+ * @phyaddr: On successful return, set to physical address of buffer
+ * @size: On successful return, set to the buffer size.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+static int __init tpm_get_kexec_buffer(phys_addr_t *phyaddr, size_t *size)
+{
+ unsigned long tmp_addr;
+ size_t tmp_size;
+ int ret;
+
+ ret = get_kexec_buffer("linux,tpm-kexec-buffer", _addr, _size);
+ if (ret)
+ return ret;
+
+ *phyaddr = (phys_addr_t)tmp_addr;
+ *size = tmp_size;
+
+ return 0;
+}
+
+/**
+ * tpm_of_remove_kexec_buffer - remove the linux,tpm-kexec-buffer node
+ */
+static int __init tpm_of_remove_kexec_buffer(void)
+{
+ struct property *prop;
+
+ prop = of_find_property(of_chosen, "linux,tpm-kexec-buffer", NULL);
+ if (!prop)
+ return -ENOENT;
+
+ return of_remove_property(of_chosen, prop);
+}
+
+/**
+ * remove_tpm_buffer - remove the TPM log buffer property and reservation from
@fdt
+ *
+ * @fdt: Flattened Device Tree to update
+ * @chosen_node: Offset to the chosen node in the device tree
+ *
+ * The TPM log measurement buffer is of no use to a subsequent kernel, so we
always
+ * remove it from the device tree.
+ */
+static void
[PATCH v9 3/4] of: kexec: Refactor IMA buffer related functions to make them reusable
Refactor IMA buffer related functions to make them reusable for carrying
TPM logs across kexec.
Signed-off-by: Stefan Berger
Cc: Rob Herring
Cc: Frank Rowand
Cc: Mimi Zohar
Reviewed-by: Mimi Zohar
Reviewed-by: Rob Herring
Tested-by: Nageswara R Sastry
Tested-by: Coiby Xu
---
v6:
- Add __init to get_kexec_buffer as suggested by Jonathan
v5:
- Rebased on Jonathan McDowell's commit "b69a2afd5afc x86/kexec: Carry
forward IMA measurement log on kexec"
v4:
- Move debug output into setup_buffer()
---
drivers/of/kexec.c | 126 ++---
1 file changed, 74 insertions(+), 52 deletions(-)
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 1373d7e0a9b3..fa8c0c75adf9 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -117,45 +117,57 @@ static int do_get_kexec_buffer(const void *prop, int len,
unsigned long *addr,
}
#ifdef CONFIG_HAVE_IMA_KEXEC
-/**
- * ima_get_kexec_buffer - get IMA buffer from the previous kernel
- * @addr: On successful return, set to point to the buffer contents.
- * @size: On successful return, set to the buffer size.
- *
- * Return: 0 on success, negative errno on error.
- */
-int __init ima_get_kexec_buffer(void **addr, size_t *size)
+static int __init get_kexec_buffer(const char *name, unsigned long *addr,
+ size_t *size)
{
int ret, len;
- unsigned long tmp_addr;
unsigned long start_pfn, end_pfn;
- size_t tmp_size;
const void *prop;
- prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", );
+ prop = of_get_property(of_chosen, name, );
if (!prop)
return -ENOENT;
- ret = do_get_kexec_buffer(prop, len, _addr, _size);
+ ret = do_get_kexec_buffer(prop, len, addr, size);
if (ret)
return ret;
- /* Do some sanity on the returned size for the ima-kexec buffer */
- if (!tmp_size)
+ /* Do some sanity on the returned size for the kexec buffer */
+ if (!*size)
return -ENOENT;
/*
* Calculate the PFNs for the buffer and ensure
* they are with in addressable memory.
*/
- start_pfn = PHYS_PFN(tmp_addr);
- end_pfn = PHYS_PFN(tmp_addr + tmp_size - 1);
+ start_pfn = PHYS_PFN(*addr);
+ end_pfn = PHYS_PFN(*addr + *size - 1);
if (!page_is_ram(start_pfn) || !page_is_ram(end_pfn)) {
- pr_warn("IMA buffer at 0x%lx, size = 0x%zx beyond memory\n",
- tmp_addr, tmp_size);
+ pr_warn("%s buffer at 0x%lx, size = 0x%zx beyond memory\n",
+ name, *addr, *size);
return -EINVAL;
}
+ return 0;
+}
+
+/**
+ * ima_get_kexec_buffer - get IMA buffer from the previous kernel
+ * @addr: On successful return, set to point to the buffer contents.
+ * @size: On successful return, set to the buffer size.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ int ret;
+ unsigned long tmp_addr;
+ size_t tmp_size;
+
+ ret = get_kexec_buffer("linux,ima-kexec-buffer", _addr, _size);
+ if (ret)
+ return ret;
+
*addr = __va(tmp_addr);
*size = tmp_size;
@@ -188,72 +200,82 @@ int __init ima_free_kexec_buffer(void)
}
#endif
-/**
- * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
- *
- * @fdt: Flattened Device Tree to update
- * @chosen_node: Offset to the chosen node in the device tree
- *
- * The IMA measurement buffer is of no use to a subsequent kernel, so we always
- * remove it from the device tree.
- */
-static void remove_ima_buffer(void *fdt, int chosen_node)
+static int remove_buffer(void *fdt, int chosen_node, const char *name)
{
int ret, len;
unsigned long addr;
size_t size;
const void *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return;
-
- prop = fdt_getprop(fdt, chosen_node, "linux,ima-kexec-buffer", );
+ prop = fdt_getprop(fdt, chosen_node, name, );
if (!prop)
- return;
+ return -ENOENT;
ret = do_get_kexec_buffer(prop, len, , );
- fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer");
+ fdt_delprop(fdt, chosen_node, name);
if (ret)
- return;
+ return ret;
ret = fdt_find_and_del_mem_rsv(fdt, addr, size);
if (!ret)
- pr_debug("Removed old IMA buffer reservation.\n");
+ pr_debug("Remove old %s buffer reserveration", name);
+ return ret;
}
-#ifdef CONFIG_IMA_KEXEC
/**
- * setup_ima_buffer - add IMA buffer information to the fdt
- * @image: kexec image being loaded.
- * @fdt: Flattened device tree for the next kernel.
- * @chosen_node: Offset to the chosen node.
+ *
[PATCH v9 1/4] drivers: of: kexec ima: Support 32-bit platforms
From: Palmer Dabbelt
RISC-V recently added kexec_file() support, which uses enables kexec
IMA. We're the first 32-bit platform to support this, so we found a
build bug.
Acked-by: Rob Herring
Signed-off-by: Palmer Dabbelt
Reviewed-by: Mimi Zohar
---
drivers/of/kexec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index f26d2ba8a371..1373d7e0a9b3 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -250,8 +250,8 @@ static int setup_ima_buffer(const struct kimage *image,
void *fdt,
if (ret)
return -EINVAL;
- pr_debug("IMA buffer at 0x%llx, size = 0x%zx\n",
-image->ima_buffer_addr, image->ima_buffer_size);
+ pr_debug("IMA buffer at 0x%pa, size = 0x%zx\n",
+>ima_buffer_addr, image->ima_buffer_size);
return 0;
}
--
2.38.1
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v9 2/4] tpm: of: Make of-tree specific function commonly available
Simplify tpm_read_log_of() by moving reusable parts of the code into
an inline function that makes it commonly available so it can be
used also for kexec support. Call the new of_tpm_get_sml_parameters()
function from the TPM Open Firmware driver.
Signed-off-by: Stefan Berger
Cc: Jarkko Sakkinen
Cc: Jason Gunthorpe
Cc: Rob Herring
Cc: Frank Rowand
Reviewed-by: Mimi Zohar
Tested-by: Nageswara R Sastry
Tested-by: Coiby Xu
Acked-by: Jarkko Sakkinen
---
v9:
- Rebased on 6.3-rc7; call tpm_read_log_memory_region if -ENODEV returned
v7:
- Added original comment back into inlined function
v4:
- converted to inline function
---
drivers/char/tpm/eventlog/of.c | 30 +---
include/linux/tpm.h| 36 ++
2 files changed, 41 insertions(+), 25 deletions(-)
diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c
index 930fe43d5daf..41688d9cbd3b 100644
--- a/drivers/char/tpm/eventlog/of.c
+++ b/drivers/char/tpm/eventlog/of.c
@@ -51,11 +51,10 @@ static int tpm_read_log_memory_region(struct tpm_chip *chip)
int tpm_read_log_of(struct tpm_chip *chip)
{
struct device_node *np;
- const u32 *sizep;
- const u64 *basep;
struct tpm_bios_log *log;
u32 size;
u64 base;
+ int ret;
log = >log;
if (chip->dev.parent && chip->dev.parent->of_node)
@@ -66,30 +65,11 @@ int tpm_read_log_of(struct tpm_chip *chip)
if (of_property_read_bool(np, "powered-while-suspended"))
chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
- sizep = of_get_property(np, "linux,sml-size", NULL);
- basep = of_get_property(np, "linux,sml-base", NULL);
- if (sizep == NULL && basep == NULL)
+ ret = of_tpm_get_sml_parameters(np, , );
+ if (ret == -ENODEV)
return tpm_read_log_memory_region(chip);
- if (sizep == NULL || basep == NULL)
- return -EIO;
-
- /*
-* For both vtpm/tpm, firmware has log addr and log size in big
-* endian format. But in case of vtpm, there is a method called
-* sml-handover which is run during kernel init even before
-* device tree is setup. This sml-handover function takes care
-* of endianness and writes to sml-base and sml-size in little
-* endian format. For this reason, vtpm doesn't need conversion
-* but physical tpm needs the conversion.
-*/
- if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
- of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
- size = be32_to_cpup((__force __be32 *)sizep);
- base = be64_to_cpup((__force __be64 *)basep);
- } else {
- size = *sizep;
- base = *basep;
- }
+ if (ret < 0)
+ return ret;
if (size == 0) {
dev_warn(>dev, "%s: Event log area empty\n", __func__);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 4dc97b9f65fb..dd9a376a1dbb 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -461,4 +461,40 @@ static inline struct tpm_chip *tpm_default_chip(void)
return NULL;
}
#endif
+
+#ifdef CONFIG_OF
+static inline int of_tpm_get_sml_parameters(struct device_node *np,
+ u64 *base, u32 *size)
+{
+ const u32 *sizep;
+ const u64 *basep;
+
+ sizep = of_get_property(np, "linux,sml-size", NULL);
+ basep = of_get_property(np, "linux,sml-base", NULL);
+ if (sizep == NULL && basep == NULL)
+ return -ENODEV;
+ if (sizep == NULL || basep == NULL)
+ return -EIO;
+
+ /*
+* For both vtpm/tpm, firmware has log addr and log size in big
+* endian format. But in case of vtpm, there is a method called
+* sml-handover which is run during kernel init even before
+* device tree is setup. This sml-handover function takes care
+* of endianness and writes to sml-base and sml-size in little
+* endian format. For this reason, vtpm doesn't need conversion
+* but physical tpm needs the conversion.
+*/
+ if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
+ of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
+ *size = be32_to_cpup((__force __be32 *)sizep);
+ *base = be64_to_cpup((__force __be64 *)basep);
+ } else {
+ *size = *sizep;
+ *base = *basep;
+ }
+ return 0;
+}
+#endif
+
#endif
--
2.38.1
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v9 0/4] tpm: Preserve TPM measurement log across kexec (ppc64)
The of-tree subsystem does not currently preserve the IBM vTPM 1.2 and vTPM 2.0 measurement logs across a kexec on PowerVM and PowerKVM. This series fixes this for the kexec_file_load() syscall using the flattened device tree (fdt) to carry the TPM measurement log's buffer across kexec. Stefan v9: - 2/4: Rebased on 6.3-rc7: call tpm_read_log_memory_region if -ENODEV returned v8: - Added Jarkko's, Coiby's, and Rob's tags - Rebase on v6.0-rc3 that absorbed 2 already upstreamed patches v7: - Added Nageswara's Tested-by tags - Added back original comment to inline function and removed Jarkko's R-b tag v6: - Add __init to get_kexec_buffer as suggested by Jonathan - Fixed issue detected by kernel test robot v5: - Rebased on 1 more patch that would otherwise create merge conflicts v4: - Rebased on 2 patches that would otherwise create merge conflicts; posting these patches in this series with several tags removed so krobot can test the series already - Changes to individual patches documented in patch descripitons v3: - Moved TPM Open Firmware related function to drivers/char/tpm/eventlog/tpm_of.c v2: - rearranged patches - fixed compilation issues for x86 Palmer Dabbelt (1): drivers: of: kexec ima: Support 32-bit platforms Stefan Berger (3): tpm: of: Make of-tree specific function commonly available of: kexec: Refactor IMA buffer related functions to make them reusable tpm/kexec: Duplicate TPM measurement log in of-tree for kexec drivers/char/tpm/eventlog/of.c | 30 +-- drivers/of/kexec.c | 336 - include/linux/kexec.h | 6 + include/linux/of.h | 6 + include/linux/tpm.h| 36 kernel/kexec_file.c| 6 + 6 files changed, 344 insertions(+), 76 deletions(-) base-commit: 6a8f57ae2eb07ab39a6f0ccad60c760743051026 -- 2.38.1 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 6.2 092/139] asymmetric_keys: log on fatal failures in PE/pkcs7
From: Robbie Harwood
[ Upstream commit 3584c1dbfffdabf8e3dc1dd25748bb38dd01cd43 ]
These particular errors can be encountered while trying to kexec when
secureboot lockdown is in place. Without this change, even with a
signed debug build, one still needs to reboot the machine to add the
appropriate dyndbg parameters (since lockdown blocks debugfs).
Accordingly, upgrade all pr_debug() before fatal error into pr_warn().
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link: https://lore.kernel.org/r/20230220171254.592347-3-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/pkcs7_verify.c | 10 +-
crypto/asymmetric_keys/verify_pefile.c | 24
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c
b/crypto/asymmetric_keys/pkcs7_verify.c
index f6321c785714c..3da32813e4412 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -79,16 +79,16 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
}
if (sinfo->msgdigest_len != sig->digest_size) {
- pr_debug("Sig %u: Invalid digest size (%u)\n",
-sinfo->index, sinfo->msgdigest_len);
+ pr_warn("Sig %u: Invalid digest size (%u)\n",
+ sinfo->index, sinfo->msgdigest_len);
ret = -EBADMSG;
goto error;
}
if (memcmp(sig->digest, sinfo->msgdigest,
sinfo->msgdigest_len) != 0) {
- pr_debug("Sig %u: Message digest doesn't match\n",
-sinfo->index);
+ pr_warn("Sig %u: Message digest doesn't match\n",
+ sinfo->index);
ret = -EKEYREJECTED;
goto error;
}
@@ -478,7 +478,7 @@ int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
const void *data, size_t datalen)
{
if (pkcs7->data) {
- pr_debug("Data already supplied\n");
+ pr_warn("Data already supplied\n");
return -EINVAL;
}
pkcs7->data = data;
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index fe1bb374239d7..22beaf2213a22 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -74,7 +74,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
break;
default:
- pr_debug("Unknown PEOPT magic = %04hx\n", pe32->magic);
+ pr_warn("Unknown PEOPT magic = %04hx\n", pe32->magic);
return -ELIBBAD;
}
@@ -95,7 +95,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
ctx->certs_size = ddir->certs.size;
if (!ddir->certs.virtual_address || !ddir->certs.size) {
- pr_debug("Unsigned PE binary\n");
+ pr_warn("Unsigned PE binary\n");
return -ENODATA;
}
@@ -127,7 +127,7 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
unsigned len;
if (ctx->sig_len < sizeof(wrapper)) {
- pr_debug("Signature wrapper too short\n");
+ pr_warn("Signature wrapper too short\n");
return -ELIBBAD;
}
@@ -142,16 +142,16 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
* rounded up since 0.110.
*/
if (wrapper.length > ctx->sig_len) {
- pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
-ctx->sig_len, wrapper.length);
+ pr_warn("Signature wrapper bigger than sig len (%x > %x)\n",
+ ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
- pr_debug("Signature is not revision 2.0\n");
+ pr_warn("Signature is not revision 2.0\n");
return -ENOTSUPP;
}
if (wrapper.cert_type != WIN_CERT_TYPE_PKCS_SIGNED_DATA) {
- pr_debug("Signature certificate type is not PKCS\n");
+ pr_warn("Signature certificate type is not PKCS\n");
return -ENOTSUPP;
}
@@ -164,7 +164,7 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
ctx->sig_offset += sizeof(wrapper);
ctx->sig_len -= sizeof(wrapper);
if (ctx->sig_len < 4) {
- pr_debug("Signature data missing\n");
+ pr_warn("Signature data missing\n");
return -EKEYREJECTED;
}
@@
[PATCH 6.2 091/139] verify_pefile: relax wrapper length check
From: Robbie Harwood
[ Upstream commit 4fc5c74dde69a7eda172514aaeb5a7df3600adb3 ]
The PE Format Specification (section "The Attribute Certificate Table
(Image Only)") states that `dwLength` is to be rounded up to 8-byte
alignment when used for traversal. Therefore, the field is not required
to be an 8-byte multiple in the first place.
Accordingly, pesign has not performed this alignment since version
0.110. This causes kexec failure on pesign'd binaries with "PEFILE:
Signature wrapper len wrong". Update the comment and relax the check.
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link:
https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-attribute-certificate-table-image-only
Link: https://github.com/rhboot/pesign
Link: https://lore.kernel.org/r/20230220171254.592347-2-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/verify_pefile.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index 7553ab18db898..fe1bb374239d7 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -135,11 +135,15 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
pr_debug("sig wrapper = { %x, %x, %x }\n",
wrapper.length, wrapper.revision, wrapper.cert_type);
- /* Both pesign and sbsign round up the length of certificate table
-* (in optional header data directories) to 8 byte alignment.
+ /* sbsign rounds up the length of certificate table (in optional
+* header data directories) to 8 byte alignment. However, the PE
+* specification states that while entries are 8-byte aligned, this is
+* not included in their length, and as a result, pesign has not
+* rounded up since 0.110.
*/
- if (round_up(wrapper.length, 8) != ctx->sig_len) {
- pr_debug("Signature wrapper len wrong\n");
+ if (wrapper.length > ctx->sig_len) {
+ pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
+ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
--
2.39.2
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 6.1 086/134] asymmetric_keys: log on fatal failures in PE/pkcs7
From: Robbie Harwood
[ Upstream commit 3584c1dbfffdabf8e3dc1dd25748bb38dd01cd43 ]
These particular errors can be encountered while trying to kexec when
secureboot lockdown is in place. Without this change, even with a
signed debug build, one still needs to reboot the machine to add the
appropriate dyndbg parameters (since lockdown blocks debugfs).
Accordingly, upgrade all pr_debug() before fatal error into pr_warn().
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link: https://lore.kernel.org/r/20230220171254.592347-3-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/pkcs7_verify.c | 10 +-
crypto/asymmetric_keys/verify_pefile.c | 24
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c
b/crypto/asymmetric_keys/pkcs7_verify.c
index f6321c785714c..3da32813e4412 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -79,16 +79,16 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
}
if (sinfo->msgdigest_len != sig->digest_size) {
- pr_debug("Sig %u: Invalid digest size (%u)\n",
-sinfo->index, sinfo->msgdigest_len);
+ pr_warn("Sig %u: Invalid digest size (%u)\n",
+ sinfo->index, sinfo->msgdigest_len);
ret = -EBADMSG;
goto error;
}
if (memcmp(sig->digest, sinfo->msgdigest,
sinfo->msgdigest_len) != 0) {
- pr_debug("Sig %u: Message digest doesn't match\n",
-sinfo->index);
+ pr_warn("Sig %u: Message digest doesn't match\n",
+ sinfo->index);
ret = -EKEYREJECTED;
goto error;
}
@@ -478,7 +478,7 @@ int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
const void *data, size_t datalen)
{
if (pkcs7->data) {
- pr_debug("Data already supplied\n");
+ pr_warn("Data already supplied\n");
return -EINVAL;
}
pkcs7->data = data;
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index fe1bb374239d7..22beaf2213a22 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -74,7 +74,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
break;
default:
- pr_debug("Unknown PEOPT magic = %04hx\n", pe32->magic);
+ pr_warn("Unknown PEOPT magic = %04hx\n", pe32->magic);
return -ELIBBAD;
}
@@ -95,7 +95,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
ctx->certs_size = ddir->certs.size;
if (!ddir->certs.virtual_address || !ddir->certs.size) {
- pr_debug("Unsigned PE binary\n");
+ pr_warn("Unsigned PE binary\n");
return -ENODATA;
}
@@ -127,7 +127,7 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
unsigned len;
if (ctx->sig_len < sizeof(wrapper)) {
- pr_debug("Signature wrapper too short\n");
+ pr_warn("Signature wrapper too short\n");
return -ELIBBAD;
}
@@ -142,16 +142,16 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
* rounded up since 0.110.
*/
if (wrapper.length > ctx->sig_len) {
- pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
-ctx->sig_len, wrapper.length);
+ pr_warn("Signature wrapper bigger than sig len (%x > %x)\n",
+ ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
- pr_debug("Signature is not revision 2.0\n");
+ pr_warn("Signature is not revision 2.0\n");
return -ENOTSUPP;
}
if (wrapper.cert_type != WIN_CERT_TYPE_PKCS_SIGNED_DATA) {
- pr_debug("Signature certificate type is not PKCS\n");
+ pr_warn("Signature certificate type is not PKCS\n");
return -ENOTSUPP;
}
@@ -164,7 +164,7 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
ctx->sig_offset += sizeof(wrapper);
ctx->sig_len -= sizeof(wrapper);
if (ctx->sig_len < 4) {
- pr_debug("Signature data missing\n");
+ pr_warn("Signature data missing\n");
return -EKEYREJECTED;
}
@@
[PATCH 6.1 085/134] verify_pefile: relax wrapper length check
From: Robbie Harwood
[ Upstream commit 4fc5c74dde69a7eda172514aaeb5a7df3600adb3 ]
The PE Format Specification (section "The Attribute Certificate Table
(Image Only)") states that `dwLength` is to be rounded up to 8-byte
alignment when used for traversal. Therefore, the field is not required
to be an 8-byte multiple in the first place.
Accordingly, pesign has not performed this alignment since version
0.110. This causes kexec failure on pesign'd binaries with "PEFILE:
Signature wrapper len wrong". Update the comment and relax the check.
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link:
https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-attribute-certificate-table-image-only
Link: https://github.com/rhboot/pesign
Link: https://lore.kernel.org/r/20230220171254.592347-2-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/verify_pefile.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index 7553ab18db898..fe1bb374239d7 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -135,11 +135,15 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
pr_debug("sig wrapper = { %x, %x, %x }\n",
wrapper.length, wrapper.revision, wrapper.cert_type);
- /* Both pesign and sbsign round up the length of certificate table
-* (in optional header data directories) to 8 byte alignment.
+ /* sbsign rounds up the length of certificate table (in optional
+* header data directories) to 8 byte alignment. However, the PE
+* specification states that while entries are 8-byte aligned, this is
+* not included in their length, and as a result, pesign has not
+* rounded up since 0.110.
*/
- if (round_up(wrapper.length, 8) != ctx->sig_len) {
- pr_debug("Signature wrapper len wrong\n");
+ if (wrapper.length > ctx->sig_len) {
+ pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
+ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
--
2.39.2
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 5.15 45/91] verify_pefile: relax wrapper length check
From: Robbie Harwood
[ Upstream commit 4fc5c74dde69a7eda172514aaeb5a7df3600adb3 ]
The PE Format Specification (section "The Attribute Certificate Table
(Image Only)") states that `dwLength` is to be rounded up to 8-byte
alignment when used for traversal. Therefore, the field is not required
to be an 8-byte multiple in the first place.
Accordingly, pesign has not performed this alignment since version
0.110. This causes kexec failure on pesign'd binaries with "PEFILE:
Signature wrapper len wrong". Update the comment and relax the check.
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link:
https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-attribute-certificate-table-image-only
Link: https://github.com/rhboot/pesign
Link: https://lore.kernel.org/r/20230220171254.592347-2-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/verify_pefile.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index 7553ab18db898..fe1bb374239d7 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -135,11 +135,15 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
pr_debug("sig wrapper = { %x, %x, %x }\n",
wrapper.length, wrapper.revision, wrapper.cert_type);
- /* Both pesign and sbsign round up the length of certificate table
-* (in optional header data directories) to 8 byte alignment.
+ /* sbsign rounds up the length of certificate table (in optional
+* header data directories) to 8 byte alignment. However, the PE
+* specification states that while entries are 8-byte aligned, this is
+* not included in their length, and as a result, pesign has not
+* rounded up since 0.110.
*/
- if (round_up(wrapper.length, 8) != ctx->sig_len) {
- pr_debug("Signature wrapper len wrong\n");
+ if (wrapper.length > ctx->sig_len) {
+ pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
+ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
--
2.39.2
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 5.15 46/91] asymmetric_keys: log on fatal failures in PE/pkcs7
From: Robbie Harwood
[ Upstream commit 3584c1dbfffdabf8e3dc1dd25748bb38dd01cd43 ]
These particular errors can be encountered while trying to kexec when
secureboot lockdown is in place. Without this change, even with a
signed debug build, one still needs to reboot the machine to add the
appropriate dyndbg parameters (since lockdown blocks debugfs).
Accordingly, upgrade all pr_debug() before fatal error into pr_warn().
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link: https://lore.kernel.org/r/20230220171254.592347-3-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/pkcs7_verify.c | 10 +-
crypto/asymmetric_keys/verify_pefile.c | 24
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c
b/crypto/asymmetric_keys/pkcs7_verify.c
index f94a1d1ad3a6c..df279538cead3 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -79,16 +79,16 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
}
if (sinfo->msgdigest_len != sig->digest_size) {
- pr_debug("Sig %u: Invalid digest size (%u)\n",
-sinfo->index, sinfo->msgdigest_len);
+ pr_warn("Sig %u: Invalid digest size (%u)\n",
+ sinfo->index, sinfo->msgdigest_len);
ret = -EBADMSG;
goto error;
}
if (memcmp(sig->digest, sinfo->msgdigest,
sinfo->msgdigest_len) != 0) {
- pr_debug("Sig %u: Message digest doesn't match\n",
-sinfo->index);
+ pr_warn("Sig %u: Message digest doesn't match\n",
+ sinfo->index);
ret = -EKEYREJECTED;
goto error;
}
@@ -481,7 +481,7 @@ int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
const void *data, size_t datalen)
{
if (pkcs7->data) {
- pr_debug("Data already supplied\n");
+ pr_warn("Data already supplied\n");
return -EINVAL;
}
pkcs7->data = data;
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index fe1bb374239d7..22beaf2213a22 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -74,7 +74,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
break;
default:
- pr_debug("Unknown PEOPT magic = %04hx\n", pe32->magic);
+ pr_warn("Unknown PEOPT magic = %04hx\n", pe32->magic);
return -ELIBBAD;
}
@@ -95,7 +95,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
ctx->certs_size = ddir->certs.size;
if (!ddir->certs.virtual_address || !ddir->certs.size) {
- pr_debug("Unsigned PE binary\n");
+ pr_warn("Unsigned PE binary\n");
return -ENODATA;
}
@@ -127,7 +127,7 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
unsigned len;
if (ctx->sig_len < sizeof(wrapper)) {
- pr_debug("Signature wrapper too short\n");
+ pr_warn("Signature wrapper too short\n");
return -ELIBBAD;
}
@@ -142,16 +142,16 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
* rounded up since 0.110.
*/
if (wrapper.length > ctx->sig_len) {
- pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
-ctx->sig_len, wrapper.length);
+ pr_warn("Signature wrapper bigger than sig len (%x > %x)\n",
+ ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
- pr_debug("Signature is not revision 2.0\n");
+ pr_warn("Signature is not revision 2.0\n");
return -ENOTSUPP;
}
if (wrapper.cert_type != WIN_CERT_TYPE_PKCS_SIGNED_DATA) {
- pr_debug("Signature certificate type is not PKCS\n");
+ pr_warn("Signature certificate type is not PKCS\n");
return -ENOTSUPP;
}
@@ -164,7 +164,7 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
ctx->sig_offset += sizeof(wrapper);
ctx->sig_len -= sizeof(wrapper);
if (ctx->sig_len < 4) {
- pr_debug("Signature data missing\n");
+ pr_warn("Signature data missing\n");
return -EKEYREJECTED;
}
@@
[PATCH 5.10 092/124] asymmetric_keys: log on fatal failures in PE/pkcs7
From: Robbie Harwood
[ Upstream commit 3584c1dbfffdabf8e3dc1dd25748bb38dd01cd43 ]
These particular errors can be encountered while trying to kexec when
secureboot lockdown is in place. Without this change, even with a
signed debug build, one still needs to reboot the machine to add the
appropriate dyndbg parameters (since lockdown blocks debugfs).
Accordingly, upgrade all pr_debug() before fatal error into pr_warn().
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link: https://lore.kernel.org/r/20230220171254.592347-3-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/pkcs7_verify.c | 10 +-
crypto/asymmetric_keys/verify_pefile.c | 24
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c
b/crypto/asymmetric_keys/pkcs7_verify.c
index ce49820caa97f..01e54450c846f 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -79,16 +79,16 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
}
if (sinfo->msgdigest_len != sig->digest_size) {
- pr_debug("Sig %u: Invalid digest size (%u)\n",
-sinfo->index, sinfo->msgdigest_len);
+ pr_warn("Sig %u: Invalid digest size (%u)\n",
+ sinfo->index, sinfo->msgdigest_len);
ret = -EBADMSG;
goto error;
}
if (memcmp(sig->digest, sinfo->msgdigest,
sinfo->msgdigest_len) != 0) {
- pr_debug("Sig %u: Message digest doesn't match\n",
-sinfo->index);
+ pr_warn("Sig %u: Message digest doesn't match\n",
+ sinfo->index);
ret = -EKEYREJECTED;
goto error;
}
@@ -488,7 +488,7 @@ int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
const void *data, size_t datalen)
{
if (pkcs7->data) {
- pr_debug("Data already supplied\n");
+ pr_warn("Data already supplied\n");
return -EINVAL;
}
pkcs7->data = data;
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index fe1bb374239d7..22beaf2213a22 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -74,7 +74,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
break;
default:
- pr_debug("Unknown PEOPT magic = %04hx\n", pe32->magic);
+ pr_warn("Unknown PEOPT magic = %04hx\n", pe32->magic);
return -ELIBBAD;
}
@@ -95,7 +95,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
ctx->certs_size = ddir->certs.size;
if (!ddir->certs.virtual_address || !ddir->certs.size) {
- pr_debug("Unsigned PE binary\n");
+ pr_warn("Unsigned PE binary\n");
return -ENODATA;
}
@@ -127,7 +127,7 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
unsigned len;
if (ctx->sig_len < sizeof(wrapper)) {
- pr_debug("Signature wrapper too short\n");
+ pr_warn("Signature wrapper too short\n");
return -ELIBBAD;
}
@@ -142,16 +142,16 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
* rounded up since 0.110.
*/
if (wrapper.length > ctx->sig_len) {
- pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
-ctx->sig_len, wrapper.length);
+ pr_warn("Signature wrapper bigger than sig len (%x > %x)\n",
+ ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
- pr_debug("Signature is not revision 2.0\n");
+ pr_warn("Signature is not revision 2.0\n");
return -ENOTSUPP;
}
if (wrapper.cert_type != WIN_CERT_TYPE_PKCS_SIGNED_DATA) {
- pr_debug("Signature certificate type is not PKCS\n");
+ pr_warn("Signature certificate type is not PKCS\n");
return -ENOTSUPP;
}
@@ -164,7 +164,7 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
ctx->sig_offset += sizeof(wrapper);
ctx->sig_len -= sizeof(wrapper);
if (ctx->sig_len < 4) {
- pr_debug("Signature data missing\n");
+ pr_warn("Signature data missing\n");
return -EKEYREJECTED;
}
@@
[PATCH 5.10 091/124] verify_pefile: relax wrapper length check
From: Robbie Harwood
[ Upstream commit 4fc5c74dde69a7eda172514aaeb5a7df3600adb3 ]
The PE Format Specification (section "The Attribute Certificate Table
(Image Only)") states that `dwLength` is to be rounded up to 8-byte
alignment when used for traversal. Therefore, the field is not required
to be an 8-byte multiple in the first place.
Accordingly, pesign has not performed this alignment since version
0.110. This causes kexec failure on pesign'd binaries with "PEFILE:
Signature wrapper len wrong". Update the comment and relax the check.
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link:
https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-attribute-certificate-table-image-only
Link: https://github.com/rhboot/pesign
Link: https://lore.kernel.org/r/20230220171254.592347-2-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/verify_pefile.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index 7553ab18db898..fe1bb374239d7 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -135,11 +135,15 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
pr_debug("sig wrapper = { %x, %x, %x }\n",
wrapper.length, wrapper.revision, wrapper.cert_type);
- /* Both pesign and sbsign round up the length of certificate table
-* (in optional header data directories) to 8 byte alignment.
+ /* sbsign rounds up the length of certificate table (in optional
+* header data directories) to 8 byte alignment. However, the PE
+* specification states that while entries are 8-byte aligned, this is
+* not included in their length, and as a result, pesign has not
+* rounded up since 0.110.
*/
- if (round_up(wrapper.length, 8) != ctx->sig_len) {
- pr_debug("Signature wrapper len wrong\n");
+ if (wrapper.length > ctx->sig_len) {
+ pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
+ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
--
2.39.2
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 5.4 68/92] verify_pefile: relax wrapper length check
From: Robbie Harwood
[ Upstream commit 4fc5c74dde69a7eda172514aaeb5a7df3600adb3 ]
The PE Format Specification (section "The Attribute Certificate Table
(Image Only)") states that `dwLength` is to be rounded up to 8-byte
alignment when used for traversal. Therefore, the field is not required
to be an 8-byte multiple in the first place.
Accordingly, pesign has not performed this alignment since version
0.110. This causes kexec failure on pesign'd binaries with "PEFILE:
Signature wrapper len wrong". Update the comment and relax the check.
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link:
https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-attribute-certificate-table-image-only
Link: https://github.com/rhboot/pesign
Link: https://lore.kernel.org/r/20230220171254.592347-2-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/verify_pefile.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index cc9dbcecaacaa..c43b077ba37db 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -135,11 +135,15 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
pr_debug("sig wrapper = { %x, %x, %x }\n",
wrapper.length, wrapper.revision, wrapper.cert_type);
- /* Both pesign and sbsign round up the length of certificate table
-* (in optional header data directories) to 8 byte alignment.
+ /* sbsign rounds up the length of certificate table (in optional
+* header data directories) to 8 byte alignment. However, the PE
+* specification states that while entries are 8-byte aligned, this is
+* not included in their length, and as a result, pesign has not
+* rounded up since 0.110.
*/
- if (round_up(wrapper.length, 8) != ctx->sig_len) {
- pr_debug("Signature wrapper len wrong\n");
+ if (wrapper.length > ctx->sig_len) {
+ pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
+ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
--
2.39.2
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 5.4 69/92] asymmetric_keys: log on fatal failures in PE/pkcs7
From: Robbie Harwood
[ Upstream commit 3584c1dbfffdabf8e3dc1dd25748bb38dd01cd43 ]
These particular errors can be encountered while trying to kexec when
secureboot lockdown is in place. Without this change, even with a
signed debug build, one still needs to reboot the machine to add the
appropriate dyndbg parameters (since lockdown blocks debugfs).
Accordingly, upgrade all pr_debug() before fatal error into pr_warn().
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link: https://lore.kernel.org/r/20230220171254.592347-3-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/pkcs7_verify.c | 10 +-
crypto/asymmetric_keys/verify_pefile.c | 24
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c
b/crypto/asymmetric_keys/pkcs7_verify.c
index ce49820caa97f..01e54450c846f 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -79,16 +79,16 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
}
if (sinfo->msgdigest_len != sig->digest_size) {
- pr_debug("Sig %u: Invalid digest size (%u)\n",
-sinfo->index, sinfo->msgdigest_len);
+ pr_warn("Sig %u: Invalid digest size (%u)\n",
+ sinfo->index, sinfo->msgdigest_len);
ret = -EBADMSG;
goto error;
}
if (memcmp(sig->digest, sinfo->msgdigest,
sinfo->msgdigest_len) != 0) {
- pr_debug("Sig %u: Message digest doesn't match\n",
-sinfo->index);
+ pr_warn("Sig %u: Message digest doesn't match\n",
+ sinfo->index);
ret = -EKEYREJECTED;
goto error;
}
@@ -488,7 +488,7 @@ int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
const void *data, size_t datalen)
{
if (pkcs7->data) {
- pr_debug("Data already supplied\n");
+ pr_warn("Data already supplied\n");
return -EINVAL;
}
pkcs7->data = data;
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index c43b077ba37db..0701bb161b63f 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -74,7 +74,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
break;
default:
- pr_debug("Unknown PEOPT magic = %04hx\n", pe32->magic);
+ pr_warn("Unknown PEOPT magic = %04hx\n", pe32->magic);
return -ELIBBAD;
}
@@ -95,7 +95,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned
int pelen,
ctx->certs_size = ddir->certs.size;
if (!ddir->certs.virtual_address || !ddir->certs.size) {
- pr_debug("Unsigned PE binary\n");
+ pr_warn("Unsigned PE binary\n");
return -ENODATA;
}
@@ -127,7 +127,7 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
unsigned len;
if (ctx->sig_len < sizeof(wrapper)) {
- pr_debug("Signature wrapper too short\n");
+ pr_warn("Signature wrapper too short\n");
return -ELIBBAD;
}
@@ -142,16 +142,16 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
* rounded up since 0.110.
*/
if (wrapper.length > ctx->sig_len) {
- pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
-ctx->sig_len, wrapper.length);
+ pr_warn("Signature wrapper bigger than sig len (%x > %x)\n",
+ ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
- pr_debug("Signature is not revision 2.0\n");
+ pr_warn("Signature is not revision 2.0\n");
return -ENOTSUPP;
}
if (wrapper.cert_type != WIN_CERT_TYPE_PKCS_SIGNED_DATA) {
- pr_debug("Signature certificate type is not PKCS\n");
+ pr_warn("Signature certificate type is not PKCS\n");
return -ENOTSUPP;
}
@@ -164,7 +164,7 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
ctx->sig_offset += sizeof(wrapper);
ctx->sig_len -= sizeof(wrapper);
if (ctx->sig_len < 4) {
- pr_debug("Signature data missing\n");
+ pr_warn("Signature data missing\n");
return -EKEYREJECTED;
}
@@
[PATCH 4.19 45/57] verify_pefile: relax wrapper length check
From: Robbie Harwood
[ Upstream commit 4fc5c74dde69a7eda172514aaeb5a7df3600adb3 ]
The PE Format Specification (section "The Attribute Certificate Table
(Image Only)") states that `dwLength` is to be rounded up to 8-byte
alignment when used for traversal. Therefore, the field is not required
to be an 8-byte multiple in the first place.
Accordingly, pesign has not performed this alignment since version
0.110. This causes kexec failure on pesign'd binaries with "PEFILE:
Signature wrapper len wrong". Update the comment and relax the check.
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link:
https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-attribute-certificate-table-image-only
Link: https://github.com/rhboot/pesign
Link: https://lore.kernel.org/r/20230220171254.592347-2-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/verify_pefile.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index d178650fd524c..411977947adbe 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -139,11 +139,15 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
pr_debug("sig wrapper = { %x, %x, %x }\n",
wrapper.length, wrapper.revision, wrapper.cert_type);
- /* Both pesign and sbsign round up the length of certificate table
-* (in optional header data directories) to 8 byte alignment.
+ /* sbsign rounds up the length of certificate table (in optional
+* header data directories) to 8 byte alignment. However, the PE
+* specification states that while entries are 8-byte aligned, this is
+* not included in their length, and as a result, pesign has not
+* rounded up since 0.110.
*/
- if (round_up(wrapper.length, 8) != ctx->sig_len) {
- pr_debug("Signature wrapper len wrong\n");
+ if (wrapper.length > ctx->sig_len) {
+ pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
+ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
--
2.39.2
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 4.14 30/37] verify_pefile: relax wrapper length check
From: Robbie Harwood
[ Upstream commit 4fc5c74dde69a7eda172514aaeb5a7df3600adb3 ]
The PE Format Specification (section "The Attribute Certificate Table
(Image Only)") states that `dwLength` is to be rounded up to 8-byte
alignment when used for traversal. Therefore, the field is not required
to be an 8-byte multiple in the first place.
Accordingly, pesign has not performed this alignment since version
0.110. This causes kexec failure on pesign'd binaries with "PEFILE:
Signature wrapper len wrong". Update the comment and relax the check.
Signed-off-by: Robbie Harwood
Signed-off-by: David Howells
cc: Jarkko Sakkinen
cc: Eric Biederman
cc: Herbert Xu
cc: keyri...@vger.kernel.org
cc: linux-cry...@vger.kernel.org
cc: kexec@lists.infradead.org
Link:
https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-attribute-certificate-table-image-only
Link: https://github.com/rhboot/pesign
Link: https://lore.kernel.org/r/20230220171254.592347-2-rharw...@redhat.com/ #
v2
Signed-off-by: Sasha Levin
---
crypto/asymmetric_keys/verify_pefile.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/crypto/asymmetric_keys/verify_pefile.c
b/crypto/asymmetric_keys/verify_pefile.c
index d178650fd524c..411977947adbe 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -139,11 +139,15 @@ static int pefile_strip_sig_wrapper(const void *pebuf,
pr_debug("sig wrapper = { %x, %x, %x }\n",
wrapper.length, wrapper.revision, wrapper.cert_type);
- /* Both pesign and sbsign round up the length of certificate table
-* (in optional header data directories) to 8 byte alignment.
+ /* sbsign rounds up the length of certificate table (in optional
+* header data directories) to 8 byte alignment. However, the PE
+* specification states that while entries are 8-byte aligned, this is
+* not included in their length, and as a result, pesign has not
+* rounded up since 0.110.
*/
- if (round_up(wrapper.length, 8) != ctx->sig_len) {
- pr_debug("Signature wrapper len wrong\n");
+ if (wrapper.length > ctx->sig_len) {
+ pr_debug("Signature wrapper bigger than sig len (%x > %x)\n",
+ctx->sig_len, wrapper.length);
return -ELIBBAD;
}
if (wrapper.revision != WIN_CERT_REVISION_2_0) {
--
2.39.2
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH] arm64: kexec: include reboot.h
Include reboot.h in machine_kexec.c for declaration of machine_crash_shutdown. gcc-12 with W=1 reports: arch/arm64/kernel/machine_kexec.c:257:6: warning: no previous prototype for 'machine_crash_shutdown' [-Wmissing-prototypes] 257 | void machine_crash_shutdown(struct pt_regs *regs) No functional changes intended. Compile tested only. Signed-off-by: Simon Horman --- arch/arm64/kernel/machine_kexec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c index 54f8e2d081c4..078910db77a4 100644 --- a/arch/arm64/kernel/machine_kexec.c +++ b/arch/arm64/kernel/machine_kexec.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
