* [PATCH 1/6] arm64, vdso: Define vdso_{start,end} as array
2017-06-06 4:52 [PATCH] FORTIFY_SOURCE build fixes Kees Cook
@ 2017-06-06 4:52 ` Kees Cook
2017-06-06 9:43 ` Catalin Marinas
2017-06-06 4:52 ` [PATCH 2/6] efi: Avoid fortify checks in EFI stub Kees Cook
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Kees Cook @ 2017-06-06 4:52 UTC (permalink / raw)
To: Andrew Morton
Cc: Kees Cook, Catalin Marinas, Will Deacon, Jisheng Zhang, linux-kernel
Adjust vdso_{start|end} to be char arrays to avoid compile-time analysis
that flags "too large" memcmp() calls with CONFIG_FORTIFY_SOURCE.
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Jisheng Zhang <jszhang@marvell.com>
---
arch/arm64/kernel/vdso.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 41b6e31f8f55..ae35f1855e06 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -37,7 +37,7 @@
#include <asm/vdso.h>
#include <asm/vdso_datapage.h>
-extern char vdso_start, vdso_end;
+extern char vdso_start[], vdso_end[];
static unsigned long vdso_pages __ro_after_init;
/*
@@ -125,14 +125,14 @@ static int __init vdso_init(void)
struct page **vdso_pagelist;
unsigned long pfn;
- if (memcmp(&vdso_start, "\177ELF", 4)) {
+ if (memcmp(vdso_start, "\177ELF", 4)) {
pr_err("vDSO is not a valid ELF object!\n");
return -EINVAL;
}
- vdso_pages = (&vdso_end - &vdso_start) >> PAGE_SHIFT;
+ vdso_pages = (vdso_end - vdso_start) >> PAGE_SHIFT;
pr_info("vdso: %ld pages (%ld code @ %p, %ld data @ %p)\n",
- vdso_pages + 1, vdso_pages, &vdso_start, 1L, vdso_data);
+ vdso_pages + 1, vdso_pages, vdso_start, 1L, vdso_data);
/* Allocate the vDSO pagelist, plus a page for the data. */
vdso_pagelist = kcalloc(vdso_pages + 1, sizeof(struct page *),
@@ -145,7 +145,7 @@ static int __init vdso_init(void)
/* Grab the vDSO code pages. */
- pfn = sym_to_pfn(&vdso_start);
+ pfn = sym_to_pfn(vdso_start);
for (i = 0; i < vdso_pages; i++)
vdso_pagelist[i + 1] = pfn_to_page(pfn + i);
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 1/6] arm64, vdso: Define vdso_{start,end} as array
2017-06-06 4:52 ` [PATCH 1/6] arm64, vdso: Define vdso_{start,end} as array Kees Cook
@ 2017-06-06 9:43 ` Catalin Marinas
2017-06-06 9:49 ` Will Deacon
0 siblings, 1 reply; 18+ messages in thread
From: Catalin Marinas @ 2017-06-06 9:43 UTC (permalink / raw)
To: Kees Cook; +Cc: Andrew Morton, Will Deacon, Jisheng Zhang, linux-kernel
On Mon, Jun 05, 2017 at 09:52:30PM -0700, Kees Cook wrote:
> Adjust vdso_{start|end} to be char arrays to avoid compile-time analysis
> that flags "too large" memcmp() calls with CONFIG_FORTIFY_SOURCE.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Jisheng Zhang <jszhang@marvell.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 1/6] arm64, vdso: Define vdso_{start,end} as array
2017-06-06 9:43 ` Catalin Marinas
@ 2017-06-06 9:49 ` Will Deacon
2017-06-06 16:11 ` Kees Cook
0 siblings, 1 reply; 18+ messages in thread
From: Will Deacon @ 2017-06-06 9:49 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Kees Cook, Andrew Morton, Jisheng Zhang, linux-kernel
On Tue, Jun 06, 2017 at 10:43:52AM +0100, Catalin Marinas wrote:
> On Mon, Jun 05, 2017 at 09:52:30PM -0700, Kees Cook wrote:
> > Adjust vdso_{start|end} to be char arrays to avoid compile-time analysis
> > that flags "too large" memcmp() calls with CONFIG_FORTIFY_SOURCE.
> >
> > Suggested-by: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Jisheng Zhang <jszhang@marvell.com>
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Kees -- I'm assuming this series is going via some other tree, but let me
know if you want this patch to go via arm64.
Will
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 1/6] arm64, vdso: Define vdso_{start,end} as array
2017-06-06 9:49 ` Will Deacon
@ 2017-06-06 16:11 ` Kees Cook
0 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2017-06-06 16:11 UTC (permalink / raw)
To: Will Deacon; +Cc: Catalin Marinas, Andrew Morton, Jisheng Zhang, LKML
On Tue, Jun 6, 2017 at 2:49 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jun 06, 2017 at 10:43:52AM +0100, Catalin Marinas wrote:
>> On Mon, Jun 05, 2017 at 09:52:30PM -0700, Kees Cook wrote:
>> > Adjust vdso_{start|end} to be char arrays to avoid compile-time analysis
>> > that flags "too large" memcmp() calls with CONFIG_FORTIFY_SOURCE.
>> >
>> > Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> > Cc: Catalin Marinas <catalin.marinas@arm.com>
>> > Cc: Will Deacon <will.deacon@arm.com>
>> > Cc: Jisheng Zhang <jszhang@marvell.com>
>>
>> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>
> Kees -- I'm assuming this series is going via some other tree, but let me
> know if you want this patch to go via arm64.
Greg has picked up the staging fix, so it's fine if it goes via other
trees. I think akpm will pick up anything that is left over. So, yeah,
please take it via arm64.
(And I need to fix git send-email to actually Cc people on
"Suggested-by", etc... hmm)
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/6] efi: Avoid fortify checks in EFI stub
2017-06-06 4:52 [PATCH] FORTIFY_SOURCE build fixes Kees Cook
2017-06-06 4:52 ` [PATCH 1/6] arm64, vdso: Define vdso_{start,end} as array Kees Cook
@ 2017-06-06 4:52 ` Kees Cook
2017-06-06 17:13 ` Ard Biesheuvel
2017-06-06 4:52 ` [PATCH 3/6] x86/power/64: Use char arrays for asm function names Kees Cook
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Kees Cook @ 2017-06-06 4:52 UTC (permalink / raw)
To: Andrew Morton; +Cc: Kees Cook, Ard Biesheuvel, linux-kernel
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub
2017-06-06 4:52 ` [PATCH 2/6] efi: Avoid fortify checks in EFI stub Kees Cook
@ 2017-06-06 17:13 ` Ard Biesheuvel
2017-06-06 17:17 ` Mark Rutland
0 siblings, 1 reply; 18+ messages in thread
From: Ard Biesheuvel @ 2017-06-06 17:13 UTC (permalink / raw)
To: Kees Cook, Mark Rutland, Matt Fleming; +Cc: Andrew Morton, linux-kernel
(+ 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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub
2017-06-06 17:13 ` Ard Biesheuvel
@ 2017-06-06 17:17 ` Mark Rutland
2017-06-07 3:12 ` Kees Cook
0 siblings, 1 reply; 18+ messages in thread
From: Mark Rutland @ 2017-06-06 17:17 UTC (permalink / raw)
To: Ard Biesheuvel, Kees Cook; +Cc: Matt Fleming, Andrew Morton, linux-kernel
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub
2017-06-06 17:17 ` Mark Rutland
@ 2017-06-07 3:12 ` Kees Cook
2017-06-07 8:54 ` Ard Biesheuvel
2017-06-07 9:27 ` Mark Rutland
0 siblings, 2 replies; 18+ messages in thread
From: Kees Cook @ 2017-06-07 3:12 UTC (permalink / raw)
To: Mark Rutland; +Cc: Ard Biesheuvel, Matt Fleming, Andrew Morton, linux-kernel
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub
2017-06-07 3:12 ` Kees Cook
@ 2017-06-07 8:54 ` Ard Biesheuvel
2017-06-08 2:37 ` Kees Cook
2017-06-07 9:27 ` Mark Rutland
1 sibling, 1 reply; 18+ messages in thread
From: Ard Biesheuvel @ 2017-06-07 8:54 UTC (permalink / raw)
To: Kees Cook; +Cc: Mark Rutland, Matt Fleming, Andrew Morton, linux-kernel
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.)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub
2017-06-07 8:54 ` Ard Biesheuvel
@ 2017-06-08 2:37 ` Kees Cook
2017-06-09 9:01 ` Ard Biesheuvel
0 siblings, 1 reply; 18+ messages in thread
From: Kees Cook @ 2017-06-08 2:37 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Mark Rutland, Matt Fleming, Andrew Morton, linux-kernel, Daniel Micay
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub
2017-06-08 2:37 ` Kees Cook
@ 2017-06-09 9:01 ` Ard Biesheuvel
2017-06-16 9:14 ` Ard Biesheuvel
0 siblings, 1 reply; 18+ messages in thread
From: Ard Biesheuvel @ 2017-06-09 9:01 UTC (permalink / raw)
To: Kees Cook
Cc: Mark Rutland, Matt Fleming, Andrew Morton, linux-kernel, Daniel Micay
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub
2017-06-09 9:01 ` Ard Biesheuvel
@ 2017-06-16 9:14 ` Ard Biesheuvel
0 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2017-06-16 9:14 UTC (permalink / raw)
To: Kees Cook
Cc: Mark Rutland, Matt Fleming, Andrew Morton, linux-kernel, Daniel Micay
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?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] efi: Avoid fortify checks in EFI stub
2017-06-07 3:12 ` Kees Cook
2017-06-07 8:54 ` Ard Biesheuvel
@ 2017-06-07 9:27 ` Mark Rutland
1 sibling, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2017-06-07 9:27 UTC (permalink / raw)
To: Kees Cook; +Cc: Ard Biesheuvel, Matt Fleming, Andrew Morton, linux-kernel
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/6] x86/power/64: Use char arrays for asm function names
2017-06-06 4:52 [PATCH] FORTIFY_SOURCE build fixes Kees Cook
2017-06-06 4:52 ` [PATCH 1/6] arm64, vdso: Define vdso_{start,end} as array Kees Cook
2017-06-06 4:52 ` [PATCH 2/6] efi: Avoid fortify checks in EFI stub Kees Cook
@ 2017-06-06 4:52 ` Kees Cook
2017-06-06 4:52 ` [PATCH 4/6] kexec_file: Adjust declaration of kexec_purgatory Kees Cook
` (2 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2017-06-06 4:52 UTC (permalink / raw)
To: Andrew Morton; +Cc: Kees Cook, Daniel Micay, linux-kernel
This switches the hibernate_64.S function names into character arrays
to match other areas of the kernel where this is done (e.g., linker
scripts). Specifically this fixes a compile-time error noticed by the
future CONFIG_FORTIFY_SOURCE routines that complained about PAGE_SIZE
being copied out of the "single byte" core_restore_code variable.
Additionally drops the "acpi_save_state_mem" exern which does not
appear to be used anywhere else in the kernel.
Cc: Daniel Micay <danielmicay@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/suspend_64.h | 5 ++---
arch/x86/power/hibernate_64.c | 6 +++---
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/suspend_64.h b/arch/x86/include/asm/suspend_64.h
index 6136a18152af..2bd96b4df140 100644
--- a/arch/x86/include/asm/suspend_64.h
+++ b/arch/x86/include/asm/suspend_64.h
@@ -42,8 +42,7 @@ struct saved_context {
set_debugreg((thread)->debugreg##register, register)
/* routines for saving/restoring kernel state */
-extern int acpi_save_state_mem(void);
-extern char core_restore_code;
-extern char restore_registers;
+extern char core_restore_code[];
+extern char restore_registers[];
#endif /* _ASM_X86_SUSPEND_64_H */
diff --git a/arch/x86/power/hibernate_64.c b/arch/x86/power/hibernate_64.c
index a6e21fee22ea..2ab1c5059a61 100644
--- a/arch/x86/power/hibernate_64.c
+++ b/arch/x86/power/hibernate_64.c
@@ -147,7 +147,7 @@ static int relocate_restore_code(void)
if (!relocated_restore_code)
return -ENOMEM;
- memcpy((void *)relocated_restore_code, &core_restore_code, PAGE_SIZE);
+ memcpy((void *)relocated_restore_code, core_restore_code, PAGE_SIZE);
/* Make the page containing the relocated code executable */
pgd = (pgd_t *)__va(read_cr3()) + pgd_index(relocated_restore_code);
@@ -292,8 +292,8 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size)
if (max_size < sizeof(struct restore_data_record))
return -EOVERFLOW;
- rdr->jump_address = (unsigned long)&restore_registers;
- rdr->jump_address_phys = __pa_symbol(&restore_registers);
+ rdr->jump_address = (unsigned long)restore_registers;
+ rdr->jump_address_phys = __pa_symbol(restore_registers);
rdr->cr3 = restore_cr3;
rdr->magic = RESTORE_MAGIC;
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 4/6] kexec_file: Adjust declaration of kexec_purgatory
2017-06-06 4:52 [PATCH] FORTIFY_SOURCE build fixes Kees Cook
` (2 preceding siblings ...)
2017-06-06 4:52 ` [PATCH 3/6] x86/power/64: Use char arrays for asm function names Kees Cook
@ 2017-06-06 4:52 ` Kees Cook
2017-06-06 4:52 ` [PATCH 5/6] staging/rts5208: Fix read overflow in memcpy Kees Cook
2017-06-06 4:52 ` [PATCH 6/6] IB/rxe: Do not copy extra stack memory to skb Kees Cook
5 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2017-06-06 4:52 UTC (permalink / raw)
To: Andrew Morton; +Cc: Kees Cook, Daniel Micay, linux-kernel
Defining kexec_purgatory as a zero-length char array upsets compile
time size checking. Since this is built on a per-arch basis, define
it as an unsized char array (like is done for other similar things,
e.g. linker sections). This silences the warning generated by the future
CONFIG_FORTIFY_SOURCE, which did not like the memcmp() of a "0 byte"
array. This drops the __weak and uses an extern instead, since both
users define kexec_purgatory.
Cc: Daniel Micay <danielmicay@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
kernel/kexec_file.c | 7 -------
kernel/kexec_internal.h | 2 ++
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f78f719dae36..5710591a9b6d 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -26,13 +26,6 @@
#include <linux/vmalloc.h>
#include "kexec_internal.h"
-/*
- * Declare these symbols weak so that if architecture provides a purgatory,
- * these will be overridden.
- */
-char __weak kexec_purgatory[0];
-size_t __weak kexec_purgatory_size = 0;
-
static int kexec_calculate_store_digests(struct kimage *image);
/* Architectures can provide this probe function */
diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
index 799a8a452187..50dfcb039a41 100644
--- a/kernel/kexec_internal.h
+++ b/kernel/kexec_internal.h
@@ -17,6 +17,8 @@ extern struct mutex kexec_mutex;
#ifdef CONFIG_KEXEC_FILE
#include <linux/purgatory.h>
void kimage_file_post_load_cleanup(struct kimage *image);
+extern char kexec_purgatory[];
+extern size_t kexec_purgatory_size;
#else /* CONFIG_KEXEC_FILE */
static inline void kimage_file_post_load_cleanup(struct kimage *image) { }
#endif /* CONFIG_KEXEC_FILE */
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 5/6] staging/rts5208: Fix read overflow in memcpy
2017-06-06 4:52 [PATCH] FORTIFY_SOURCE build fixes Kees Cook
` (3 preceding siblings ...)
2017-06-06 4:52 ` [PATCH 4/6] kexec_file: Adjust declaration of kexec_purgatory Kees Cook
@ 2017-06-06 4:52 ` Kees Cook
2017-06-06 4:52 ` [PATCH 6/6] IB/rxe: Do not copy extra stack memory to skb Kees Cook
5 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2017-06-06 4:52 UTC (permalink / raw)
To: Andrew Morton
Cc: Kees Cook, Daniel Micay, Greg Kroah-Hartman, Wayne Porter, linux-kernel
From: Daniel Micay <danielmicay@gmail.com>
Noticed by FORTIFY_SOURCE, this swaps memcpy() for strncpy() to zero-value
fill the end of the buffer instead of over-reading a string from .rodata.
Signed-off-by: Daniel Micay <danielmicay@gmail.com>
[kees: wrote commit log]
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Wayne Porter <wporter82@gmail.com>
---
drivers/staging/rts5208/rtsx_scsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rts5208/rtsx_scsi.c b/drivers/staging/rts5208/rtsx_scsi.c
index a95c5de1aa00..36b5a11f21d2 100644
--- a/drivers/staging/rts5208/rtsx_scsi.c
+++ b/drivers/staging/rts5208/rtsx_scsi.c
@@ -536,7 +536,7 @@ static int inquiry(struct scsi_cmnd *srb, struct rtsx_chip *chip)
if (sendbytes > 8) {
memcpy(buf, inquiry_buf, 8);
- memcpy(buf + 8, inquiry_string, sendbytes - 8);
+ strncpy(buf + 8, inquiry_string, sendbytes - 8);
if (pro_formatter_flag) {
/* Additional Length */
buf[4] = 0x33;
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 6/6] IB/rxe: Do not copy extra stack memory to skb
2017-06-06 4:52 [PATCH] FORTIFY_SOURCE build fixes Kees Cook
` (4 preceding siblings ...)
2017-06-06 4:52 ` [PATCH 5/6] staging/rts5208: Fix read overflow in memcpy Kees Cook
@ 2017-06-06 4:52 ` Kees Cook
5 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2017-06-06 4:52 UTC (permalink / raw)
To: Andrew Morton
Cc: Kees Cook, Moni Shoua, Doug Ledford, Sean Hefty, Daniel Micay,
linux-kernel
This fixes a over-read condition detected by FORTIFY_SOURCE for this line:
memcpy(SKB_TO_PKT(skb), &ack_pkt, sizeof(skb->cb));
The error was:
In file included from ./include/linux/bitmap.h:8:0,
from ./include/linux/cpumask.h:11,
from ./include/linux/mm_types_task.h:13,
from ./include/linux/mm_types.h:4,
from ./include/linux/kmemcheck.h:4,
from ./include/linux/skbuff.h:18,
from drivers/infiniband/sw/rxe/rxe_resp.c:34:
In function 'memcpy',
inlined from 'send_atomic_ack.constprop' at drivers/infiniband/sw/rxe/rxe_resp.c:998:2,
inlined from 'acknowledge' at drivers/infiniband/sw/rxe/rxe_resp.c:1026:3,
inlined from 'rxe_responder' at drivers/infiniband/sw/rxe/rxe_resp.c:1286:10:
./include/linux/string.h:309:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
__read_overflow2();
Daniel Micay noted that struct rxe_pkt_info is 32 bytes on 32-bit
architectures, but skb->cb is still 64. The memcpy() over-reads 32
bytes. This fixes it by zeroing the unused bytes in skb->cb.
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Moni Shoua <monis@mellanox.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Daniel Micay <danielmicay@gmail.com>
---
drivers/infiniband/sw/rxe/rxe_resp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 23039768f541..be944d5aa9af 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -995,7 +995,9 @@ static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
free_rd_atomic_resource(qp, res);
rxe_advance_resp_resource(qp);
- memcpy(SKB_TO_PKT(skb), &ack_pkt, sizeof(skb->cb));
+ memcpy(SKB_TO_PKT(skb), &ack_pkt, sizeof(ack_pkt));
+ memset((unsigned char *)SKB_TO_PKT(skb) + sizeof(ack_pkt), 0,
+ sizeof(skb->cb) - sizeof(ack_pkt));
res->type = RXE_ATOMIC_MASK;
res->atomic.skb = skb;
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread