Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1658343 > unrolled thread

[PATCH 2/6] efi: Avoid fortify checks in EFI stub

Started byKees Cook <keescook@chromium.org>
First post2017-06-06 07:00 +0200
Last post2017-06-06 19:20 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 2/6] efi: Avoid fortify checks in EFI stub Kees Cook <keescook@chromium.org> - 2017-06-06 07:00 +0200
    Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub Mark Rutland <mark.rutland@arm.com> - 2017-06-06 19:20 +0200
      Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub Kees Cook <keescook@chromium.org> - 2017-06-07 05:20 +0200
        Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-06-07 11:00 +0200
        Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub Mark Rutland <mark.rutland@arm.com> - 2017-06-07 11:30 +0200
    Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-06-06 19:20 +0200

#1658343 — [PATCH 2/6] efi: Avoid fortify checks in EFI stub

FromKees Cook <keescook@chromium.org>
Date2017-06-06 07:00 +0200
Subject[PATCH 2/6] efi: Avoid fortify checks in EFI stub
Message-ID<tPeKZ-460-3@gated-at.bofh.it>
This avoids CONFIG_FORTIFY_SOURCE from being enabled during the EFI stub
build, as adding a panic() implementation may not work well. This can be
adjusted in the future.

Suggested-by: Daniel Micay <danielmicay@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc; Matt Fleming <matt@codeblueprint.co.uk>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/libstub/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index f7425960f6a5..37e24f525162 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -17,6 +17,7 @@ cflags-$(CONFIG_ARM)		:= $(subst -pg,,$(KBUILD_CFLAGS)) \
 cflags-$(CONFIG_EFI_ARMSTUB)	+= -I$(srctree)/scripts/dtc/libfdt
 
 KBUILD_CFLAGS			:= $(cflags-y) -DDISABLE_BRANCH_PROFILING \
+				   -D__NO_FORTIFY \
 				   $(call cc-option,-ffreestanding) \
 				   $(call cc-option,-fno-stack-protector)
 
-- 
2.7.4

[toc] | [next] | [standalone]


#1658959

FromMark Rutland <mark.rutland@arm.com>
Date2017-06-06 19:20 +0200
Message-ID<tPqj7-3hY-7@gated-at.bofh.it>
In reply to#1658343
On Tue, Jun 06, 2017 at 05:13:07PM +0000, Ard Biesheuvel wrote:
> (+ Mark, Matt)
> 
> On 6 June 2017 at 04:52, Kees Cook <keescook@chromium.org> wrote:
> > This avoids CONFIG_FORTIFY_SOURCE from being enabled during the EFI stub
> > build, as adding a panic() implementation may not work well. This can be
> > adjusted in the future.
> >
> > Suggested-by: Daniel Micay <danielmicay@gmail.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Cc; Matt Fleming <matt@codeblueprint.co.uk>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

I believe for arm64 the immediate breakage is implicitly fixed by the
<asm/string.h> definition, but I agree it makes sense to be explicit
anyhow.

FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Kees, as an aside, do you want me to patchify the vdso fixup? Or are
you going to handle that?

Thanks,
Mark.

> > ---
> >  drivers/firmware/efi/libstub/Makefile | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> > index f7425960f6a5..37e24f525162 100644
> > --- a/drivers/firmware/efi/libstub/Makefile
> > +++ b/drivers/firmware/efi/libstub/Makefile
> > @@ -17,6 +17,7 @@ cflags-$(CONFIG_ARM)          := $(subst -pg,,$(KBUILD_CFLAGS)) \
> >  cflags-$(CONFIG_EFI_ARMSTUB)   += -I$(srctree)/scripts/dtc/libfdt
> >
> >  KBUILD_CFLAGS                  := $(cflags-y) -DDISABLE_BRANCH_PROFILING \
> > +                                  -D__NO_FORTIFY \
> >                                    $(call cc-option,-ffreestanding) \
> >                                    $(call cc-option,-fno-stack-protector)
> >
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> This is unlikely to conflict with anything going through the EFI tree,
> so feel free to queue it elsewhere.

[toc] | [prev] | [next] | [standalone]


#1659350

FromKees Cook <keescook@chromium.org>
Date2017-06-07 05:20 +0200
Message-ID<tPzFL-Xd-3@gated-at.bofh.it>
In reply to#1658959
On Tue, Jun 6, 2017 at 10:17 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Jun 06, 2017 at 05:13:07PM +0000, Ard Biesheuvel wrote:
>> (+ Mark, Matt)
>>
>> On 6 June 2017 at 04:52, Kees Cook <keescook@chromium.org> wrote:
>> > This avoids CONFIG_FORTIFY_SOURCE from being enabled during the EFI stub
>> > build, as adding a panic() implementation may not work well. This can be
>> > adjusted in the future.
>> >
>> > Suggested-by: Daniel Micay <danielmicay@gmail.com>
>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> > Cc; Matt Fleming <matt@codeblueprint.co.uk>
>> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> I believe for arm64 the immediate breakage is implicitly fixed by the
> <asm/string.h> definition, but I agree it makes sense to be explicit
> anyhow.
>
> FWIW:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
> Kees, as an aside, do you want me to patchify the vdso fixup? Or are
> you going to handle that?

I sent that separately but discovered that my invocation of git
send-email failed to include a CC to you, even though I had it listed
as Suggested-by, etc. I think it's going to get queued for the arm64
tree.

>
> Thanks,
> Mark.
>
>> > ---
>> >  drivers/firmware/efi/libstub/Makefile | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
>> > index f7425960f6a5..37e24f525162 100644
>> > --- a/drivers/firmware/efi/libstub/Makefile
>> > +++ b/drivers/firmware/efi/libstub/Makefile
>> > @@ -17,6 +17,7 @@ cflags-$(CONFIG_ARM)          := $(subst -pg,,$(KBUILD_CFLAGS)) \
>> >  cflags-$(CONFIG_EFI_ARMSTUB)   += -I$(srctree)/scripts/dtc/libfdt
>> >
>> >  KBUILD_CFLAGS                  := $(cflags-y) -DDISABLE_BRANCH_PROFILING \
>> > +                                  -D__NO_FORTIFY \
>> >                                    $(call cc-option,-ffreestanding) \
>> >                                    $(call cc-option,-fno-stack-protector)
>> >
>>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> This is unlikely to conflict with anything going through the EFI tree,
>> so feel free to queue it elsewhere.

If it can go through the EFI tree, that'd be great. Less for akpm to wrangle. :)

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]


#1659558

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-06-07 11:00 +0200
Message-ID<tPEYO-4kE-23@gated-at.bofh.it>
In reply to#1659350
On 7 June 2017 at 03:12, Kees Cook <keescook@chromium.org> wrote:
> On Tue, Jun 6, 2017 at 10:17 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>> On Tue, Jun 06, 2017 at 05:13:07PM +0000, Ard Biesheuvel wrote:
>>> (+ Mark, Matt)
>>>
>>> On 6 June 2017 at 04:52, Kees Cook <keescook@chromium.org> wrote:
>>> > This avoids CONFIG_FORTIFY_SOURCE from being enabled during the EFI stub
>>> > build, as adding a panic() implementation may not work well. This can be
>>> > adjusted in the future.
>>> >
>>> > Suggested-by: Daniel Micay <danielmicay@gmail.com>
>>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>>> > Cc; Matt Fleming <matt@codeblueprint.co.uk>
>>> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
[...]
>>>
>>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>
>>> This is unlikely to conflict with anything going through the EFI tree,
>>> so feel free to queue it elsewhere.
>
> If it can go through the EFI tree, that'd be great. Less for akpm to wrangle. :)
>

That is fine, but I'd prefer not to take a single patch out of
context. Do you have a link to the entire series? I was only cc'ed on
this patch (In the future, please cc me on the entire series in cases
such as these.)

[toc] | [prev] | [next] | [standalone]


#1659582

FromMark Rutland <mark.rutland@arm.com>
Date2017-06-07 11:30 +0200
Message-ID<tPFrP-4M0-1@gated-at.bofh.it>
In reply to#1659350
On Tue, Jun 06, 2017 at 08:12:22PM -0700, Kees Cook wrote:
> On Tue, Jun 6, 2017 at 10:17 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Kees, as an aside, do you want me to patchify the vdso fixup? Or are
> > you going to handle that?
> 
> I sent that separately but discovered that my invocation of git
> send-email failed to include a CC to you, even though I had it listed
> as Suggested-by, etc. I think it's going to get queued for the arm64
> tree.

Ah; great, thanks for handling that!

FWIW, don't worry about the Cc. You're stuck between a rock and a hard
place there, as git send-email only adds those with a "Cc: " line
(ignoring all other tags), and some people don't want to be Cc'd after
giving an ack, etc.

Selfishly, one thing that would be helpful is to Cc LAKML on pathes for
arm64. Myself and others will spot stuff that goes there, but don't
subscribe to LKML (or scan it less frequently).

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]


#1658963

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-06-06 19:20 +0200
Message-ID<tPqj7-3hY-9@gated-at.bofh.it>
In reply to#1658343
(+ Mark, Matt)

On 6 June 2017 at 04:52, Kees Cook <keescook@chromium.org> wrote:
> This avoids CONFIG_FORTIFY_SOURCE from being enabled during the EFI stub
> build, as adding a panic() implementation may not work well. This can be
> adjusted in the future.
>
> Suggested-by: Daniel Micay <danielmicay@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc; Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/firmware/efi/libstub/Makefile | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index f7425960f6a5..37e24f525162 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -17,6 +17,7 @@ cflags-$(CONFIG_ARM)          := $(subst -pg,,$(KBUILD_CFLAGS)) \
>  cflags-$(CONFIG_EFI_ARMSTUB)   += -I$(srctree)/scripts/dtc/libfdt
>
>  KBUILD_CFLAGS                  := $(cflags-y) -DDISABLE_BRANCH_PROFILING \
> +                                  -D__NO_FORTIFY \
>                                    $(call cc-option,-ffreestanding) \
>                                    $(call cc-option,-fno-stack-protector)
>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

This is unlikely to conflict with anything going through the EFI tree,
so feel free to queue it elsewhere.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web