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

2017-06-16 Thread Ard Biesheuvel
On 9 June 2017 at 11:01, Ard Biesheuvel  wrote:
> On 8 June 2017 at 02:37, Kees Cook  wrote:
>> On Wed, Jun 7, 2017 at 1:54 AM, Ard Biesheuvel
>>  wrote:
>>> On 7 June 2017 at 03:12, Kees Cook  wrote:
 On Tue, Jun 6, 2017 at 10:17 AM, Mark Rutland  wrote:
> On Tue, Jun 06, 2017 at 05:13:07PM +, Ard Biesheuvel wrote:
>> (+ Mark, Matt)
>>
>> On 6 June 2017 at 04:52, Kees Cook  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 
>> > Signed-off-by: Kees Cook 
>> > Cc; Matt Fleming 
>> > Cc: Ard Biesheuvel 
>>> [...]
>>
>> Reviewed-by: Ard Biesheuvel 
>>
>> 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?


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

2017-06-09 Thread Ard Biesheuvel
On 8 June 2017 at 02:37, Kees Cook  wrote:
> On Wed, Jun 7, 2017 at 1:54 AM, Ard Biesheuvel
>  wrote:
>> On 7 June 2017 at 03:12, Kees Cook  wrote:
>>> On Tue, Jun 6, 2017 at 10:17 AM, Mark Rutland  wrote:
 On Tue, Jun 06, 2017 at 05:13:07PM +, Ard Biesheuvel wrote:
> (+ Mark, Matt)
>
> On 6 June 2017 at 04:52, Kees Cook  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 
> > Signed-off-by: Kees Cook 
> > Cc; Matt Fleming 
> > Cc: Ard Biesheuvel 
>> [...]
>
> Reviewed-by: Ard Biesheuvel 
>
> 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


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

2017-06-07 Thread Kees Cook
On Wed, Jun 7, 2017 at 1:54 AM, Ard Biesheuvel
 wrote:
> On 7 June 2017 at 03:12, Kees Cook  wrote:
>> On Tue, Jun 6, 2017 at 10:17 AM, Mark Rutland  wrote:
>>> On Tue, Jun 06, 2017 at 05:13:07PM +, Ard Biesheuvel wrote:
 (+ Mark, Matt)

 On 6 June 2017 at 04:52, Kees Cook  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 
 > Signed-off-by: Kees Cook 
 > Cc; Matt Fleming 
 > Cc: Ard Biesheuvel 
> [...]

 Reviewed-by: Ard Biesheuvel 

 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


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

2017-06-07 Thread Mark Rutland
On Tue, Jun 06, 2017 at 08:12:22PM -0700, Kees Cook wrote:
> On Tue, Jun 6, 2017 at 10:17 AM, Mark Rutland  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.


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

2017-06-07 Thread Ard Biesheuvel
On 7 June 2017 at 03:12, Kees Cook  wrote:
> On Tue, Jun 6, 2017 at 10:17 AM, Mark Rutland  wrote:
>> On Tue, Jun 06, 2017 at 05:13:07PM +, Ard Biesheuvel wrote:
>>> (+ Mark, Matt)
>>>
>>> On 6 June 2017 at 04:52, Kees Cook  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 
>>> > Signed-off-by: Kees Cook 
>>> > Cc; Matt Fleming 
>>> > Cc: Ard Biesheuvel 
[...]
>>>
>>> Reviewed-by: Ard Biesheuvel 
>>>
>>> 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.)


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

2017-06-06 Thread Kees Cook
On Tue, Jun 6, 2017 at 10:17 AM, Mark Rutland  wrote:
> On Tue, Jun 06, 2017 at 05:13:07PM +, Ard Biesheuvel wrote:
>> (+ Mark, Matt)
>>
>> On 6 June 2017 at 04:52, Kees Cook  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 
>> > Signed-off-by: Kees Cook 
>> > Cc; Matt Fleming 
>> > Cc: Ard Biesheuvel 
>
> I believe for arm64 the immediate breakage is implicitly fixed by the
>  definition, but I agree it makes sense to be explicit
> anyhow.
>
> FWIW:
>
> Acked-by: Mark Rutland 
>
> 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 
>>
>> 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


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

2017-06-06 Thread Mark Rutland
On Tue, Jun 06, 2017 at 05:13:07PM +, Ard Biesheuvel wrote:
> (+ Mark, Matt)
> 
> On 6 June 2017 at 04:52, Kees Cook  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 
> > Signed-off-by: Kees Cook 
> > Cc; Matt Fleming 
> > Cc: Ard Biesheuvel 

I believe for arm64 the immediate breakage is implicitly fixed by the
 definition, but I agree it makes sense to be explicit
anyhow.

FWIW:

Acked-by: Mark Rutland 

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 
> 
> This is unlikely to conflict with anything going through the EFI tree,
> so feel free to queue it elsewhere.


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

2017-06-06 Thread Ard Biesheuvel
(+ Mark, Matt)

On 6 June 2017 at 04:52, Kees Cook  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 
> Signed-off-by: Kees Cook 
> Cc; Matt Fleming 
> Cc: Ard Biesheuvel 
> ---
>  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 

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


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

2017-06-05 Thread Kees Cook
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 
Signed-off-by: Kees Cook 
Cc; Matt Fleming 
Cc: Ard Biesheuvel 
---
 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