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 9 — 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 Kees Cook <keescook@chromium.org> - 2017-06-08 04:40 +0200
            Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-06-09 11:10 +0200
              Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-06-16 11:20 +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]

#1660674

FromKees Cook <keescook@chromium.org>
Date2017-06-08 04:40 +0200
Message-ID<tPVwB-6GZ-3@gated-at.bofh.it>
In reply to#1659558
On Wed, Jun 7, 2017 at 1:54 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> 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.)

This is to fix stuff noticed by the CONFIG_FORTIFY_SOURCE feature, now in -mm:
https://marc.info/?l=linux-kernel&m=149579258121273&w=2

I was originally preparing it along with various fixes in my KSPP
tree, but akpm took it into -mm instead, and asked that I send out the
remaining fixes that hadn't been picked up yet. The thread with my
sending starts here:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1413683.html

Hopefully that helps!

-Kees

-- 
Kees Cook
Pixel Security

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

#1662126

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-06-09 11:10 +0200
Message-ID<tQo5A-84n-25@gated-at.bofh.it>
In reply to#1660674
On 8 June 2017 at 02:37, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Jun 7, 2017 at 1:54 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> 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.)
>
> This is to fix stuff noticed by the CONFIG_FORTIFY_SOURCE feature, now in -mm:
> https://marc.info/?l=linux-kernel&m=149579258121273&w=2
>
> I was originally preparing it along with various fixes in my KSPP
> tree, but akpm took it into -mm instead, and asked that I send out the
> remaining fixes that hadn't been picked up yet. The thread with my
> sending starts here:
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1413683.html
>
> Hopefully that helps!
>

Thanks Kees

Queued in efi/next

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

#1667570

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-06-16 11:20 +0200
Message-ID<tSVA6-6x9-15@gated-at.bofh.it>
In reply to#1662126
On 9 June 2017 at 11:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 8 June 2017 at 02:37, Kees Cook <keescook@chromium.org> wrote:
>> On Wed, Jun 7, 2017 at 1:54 AM, Ard Biesheuvel
>> <ard.biesheuvel@linaro.org> wrote:
>>> 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.)
>>
>> This is to fix stuff noticed by the CONFIG_FORTIFY_SOURCE feature, now in -mm:
>> https://marc.info/?l=linux-kernel&m=149579258121273&w=2
>>
>> I was originally preparing it along with various fixes in my KSPP
>> tree, but akpm took it into -mm instead, and asked that I send out the
>> remaining fixes that hadn't been picked up yet. The thread with my
>> sending starts here:
>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1413683.html
>>
>> Hopefully that helps!
>>
>
> Thanks Kees
>
> Queued in efi/next

I see this has turned up in -next now. I guess I should drop it the
from the EFI tree then?

[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