Patchwork efi: Include tpm_eventlog.h after asm/efi.h to avoid memcpy breakage

login
register
mail settings
Submitter Matthew Garrett
Date April 3, 2019, 7:32 p.m.
Message ID <20190403193238.145204-1-matthewgarrett@google.com>
Download mbox | patch
Permalink /patch/765039/
State New
Headers show

Comments

Matthew Garrett - April 3, 2019, 7:32 p.m.
769a8089c1fd2 (x86, efi, kasan: #undef memset/memcpy/memmove per arch)
disables the KASAN version of certain memory calls in the EFI boot stub.
tpm_eventlog.h references memcpy, so must be included after asm/efi.h in
order to allow 769a8089c1fd2 to take effect on its declarations.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 drivers/firmware/efi/libstub/tpm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Jarkko Sakkinen - April 4, 2019, 12:42 p.m.
On Wed, Apr 03, 2019 at 12:32:37PM -0700, Matthew Garrett wrote:
> 769a8089c1fd2 (x86, efi, kasan: #undef memset/memcpy/memmove per arch)
> disables the KASAN version of certain memory calls in the EFI boot stub.
> tpm_eventlog.h references memcpy, so must be included after asm/efi.h in
> order to allow 769a8089c1fd2 to take effect on its declarations.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko
Ard Biesheuvel - April 4, 2019, 12:51 p.m.
On Wed, 3 Apr 2019 at 21:32, Matthew Garrett <matthewgarrett@google.com> wrote:
>
> 769a8089c1fd2 (x86, efi, kasan: #undef memset/memcpy/memmove per arch)
> disables the KASAN version of certain memory calls in the EFI boot stub.
> tpm_eventlog.h references memcpy, so must be included after asm/efi.h in
> order to allow 769a8089c1fd2 to take effect on its declarations.
>

I can't find any reference to memcpy in tpm_eventlog.h or tpm.h which
it includes. So can you describe the exact problem? If the issue is
due to one of the transitively included headers two levels down, then
this is fragile as hell since the same header may be included by
linux/efi.h due to, e.g.,  minor config changes.


> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
>  drivers/firmware/efi/libstub/tpm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
> index b6e93e14fcf1..777c1e82495a 100644
> --- a/drivers/firmware/efi/libstub/tpm.c
> +++ b/drivers/firmware/efi/libstub/tpm.c
> @@ -8,9 +8,11 @@
>   *     Thiebaud Weksteen <tweek@google.com>
>   */
>  #include <linux/efi.h>
> -#include <linux/tpm_eventlog.h>
>  #include <asm/efi.h>
>
> +/* Must be included after asm/efi.h in order to avoid using the wrong memcpy */
> +#include <linux/tpm_eventlog.h>
> +
>  #include "efistub.h"
>
>  #ifdef CONFIG_RESET_ATTACK_MITIGATION
> --
> 2.21.0.392.gf8f6787159e-goog
>
Jarkko Sakkinen - April 15, 2019, 8:53 a.m.
On Wed, Apr 03, 2019 at 12:32:37PM -0700, Matthew Garrett wrote:
> 769a8089c1fd2 (x86, efi, kasan: #undef memset/memcpy/memmove per arch)
> disables the KASAN version of certain memory calls in the EFI boot stub.
> tpm_eventlog.h references memcpy, so must be included after asm/efi.h in
> order to allow 769a8089c1fd2 to take effect on its declarations.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>

Applied.

/Jarkko

Patch

diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index b6e93e14fcf1..777c1e82495a 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -8,9 +8,11 @@ 
  *     Thiebaud Weksteen <tweek@google.com>
  */
 #include <linux/efi.h>
-#include <linux/tpm_eventlog.h>
 #include <asm/efi.h>
 
+/* Must be included after asm/efi.h in order to avoid using the wrong memcpy */
+#include <linux/tpm_eventlog.h>
+
 #include "efistub.h"
 
 #ifdef CONFIG_RESET_ATTACK_MITIGATION