| Message ID | 1469715314-23260-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | d6732fc402c2665f61e72faf206a0268e65236e9 |
| Headers | show |
Hi Ard, On Thu, Jul 28, 2016 at 04:15:14PM +0200, Ard Biesheuvel wrote: > Due to the untyped KIMAGE_VADDR constant, the linker may not notice > that the __rela_offset and __dynsym_offset expressions are absolute > values (i.e., are not subject to relocation). This does not matter for > KASLR, but it does confuse kallsyms in relative mode, since it uses > the lowest non-absolute symbol address as the anchor point, and expects > all other symbol addresses to be within 4 GB of it. > > Fix this by qualifying these expressions as ABSOLUTE() explicitly. > > Fixes: 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map") > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > arch/arm64/kernel/vmlinux.lds.S | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > index 89d6e177ecbd..d49492179967 100644 > --- a/arch/arm64/kernel/vmlinux.lds.S > +++ b/arch/arm64/kernel/vmlinux.lds.S > @@ -184,9 +184,9 @@ SECTIONS > *(.hash) > } > > - __rela_offset = ADDR(.rela) - KIMAGE_VADDR; > + __rela_offset = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR); > __rela_size = SIZEOF(.rela); > - __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR; > + __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR); You removed the __dynsym_offset entry entirely in "arm64: relocatable: suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part of the patch and assume you'll do a separate backport for stable. Ok? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 29 July 2016 at 11:06, Will Deacon <will.deacon@arm.com> wrote: > Hi Ard, > > On Thu, Jul 28, 2016 at 04:15:14PM +0200, Ard Biesheuvel wrote: >> Due to the untyped KIMAGE_VADDR constant, the linker may not notice >> that the __rela_offset and __dynsym_offset expressions are absolute >> values (i.e., are not subject to relocation). This does not matter for >> KASLR, but it does confuse kallsyms in relative mode, since it uses >> the lowest non-absolute symbol address as the anchor point, and expects >> all other symbol addresses to be within 4 GB of it. >> >> Fix this by qualifying these expressions as ABSOLUTE() explicitly. >> >> Fixes: 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map") >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> arch/arm64/kernel/vmlinux.lds.S | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S >> index 89d6e177ecbd..d49492179967 100644 >> --- a/arch/arm64/kernel/vmlinux.lds.S >> +++ b/arch/arm64/kernel/vmlinux.lds.S >> @@ -184,9 +184,9 @@ SECTIONS >> *(.hash) >> } >> >> - __rela_offset = ADDR(.rela) - KIMAGE_VADDR; >> + __rela_offset = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR); >> __rela_size = SIZEOF(.rela); >> - __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR; >> + __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR); > > You removed the __dynsym_offset entry entirely in "arm64: relocatable: > suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part > of the patch and assume you'll do a separate backport for stable. Ok? > If you are ok with fixing up patches manually while applying, could we just reorder them instead? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 29 July 2016 at 11:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 29 July 2016 at 11:06, Will Deacon <will.deacon@arm.com> wrote: >> Hi Ard, >> >> On Thu, Jul 28, 2016 at 04:15:14PM +0200, Ard Biesheuvel wrote: >>> Due to the untyped KIMAGE_VADDR constant, the linker may not notice >>> that the __rela_offset and __dynsym_offset expressions are absolute >>> values (i.e., are not subject to relocation). This does not matter for >>> KASLR, but it does confuse kallsyms in relative mode, since it uses >>> the lowest non-absolute symbol address as the anchor point, and expects >>> all other symbol addresses to be within 4 GB of it. >>> >>> Fix this by qualifying these expressions as ABSOLUTE() explicitly. >>> >>> Fixes: 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map") >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> --- >>> arch/arm64/kernel/vmlinux.lds.S | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S >>> index 89d6e177ecbd..d49492179967 100644 >>> --- a/arch/arm64/kernel/vmlinux.lds.S >>> +++ b/arch/arm64/kernel/vmlinux.lds.S >>> @@ -184,9 +184,9 @@ SECTIONS >>> *(.hash) >>> } >>> >>> - __rela_offset = ADDR(.rela) - KIMAGE_VADDR; >>> + __rela_offset = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR); >>> __rela_size = SIZEOF(.rela); >>> - __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR; >>> + __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR); >> >> You removed the __dynsym_offset entry entirely in "arm64: relocatable: >> suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part >> of the patch and assume you'll do a separate backport for stable. Ok? >> > > If you are ok with fixing up patches manually while applying, could we > just reorder them instead? Actually, this should go into v4.8-late not -stable _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Fri, Jul 29, 2016 at 12:46:49PM +0200, Ard Biesheuvel wrote: > On 29 July 2016 at 11:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On 29 July 2016 at 11:06, Will Deacon <will.deacon@arm.com> wrote: > >> Hi Ard, > >> > >> On Thu, Jul 28, 2016 at 04:15:14PM +0200, Ard Biesheuvel wrote: > >>> Due to the untyped KIMAGE_VADDR constant, the linker may not notice > >>> that the __rela_offset and __dynsym_offset expressions are absolute > >>> values (i.e., are not subject to relocation). This does not matter for > >>> KASLR, but it does confuse kallsyms in relative mode, since it uses > >>> the lowest non-absolute symbol address as the anchor point, and expects > >>> all other symbol addresses to be within 4 GB of it. > >>> > >>> Fix this by qualifying these expressions as ABSOLUTE() explicitly. > >>> > >>> Fixes: 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map") > >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >>> --- > >>> arch/arm64/kernel/vmlinux.lds.S | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > >>> index 89d6e177ecbd..d49492179967 100644 > >>> --- a/arch/arm64/kernel/vmlinux.lds.S > >>> +++ b/arch/arm64/kernel/vmlinux.lds.S > >>> @@ -184,9 +184,9 @@ SECTIONS > >>> *(.hash) > >>> } > >>> > >>> - __rela_offset = ADDR(.rela) - KIMAGE_VADDR; > >>> + __rela_offset = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR); > >>> __rela_size = SIZEOF(.rela); > >>> - __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR; > >>> + __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR); > >> > >> You removed the __dynsym_offset entry entirely in "arm64: relocatable: > >> suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part > >> of the patch and assume you'll do a separate backport for stable. Ok? > >> > > > > If you are ok with fixing up patches manually while applying, could we > > just reorder them instead? > > Actually, this should go into v4.8-late not -stable Then the fixes tag is pretty confusing, since that's a commit from v4.7. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 29 July 2016 at 12:49, Will Deacon <will.deacon@arm.com> wrote: > On Fri, Jul 29, 2016 at 12:46:49PM +0200, Ard Biesheuvel wrote: >> On 29 July 2016 at 11:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> > On 29 July 2016 at 11:06, Will Deacon <will.deacon@arm.com> wrote: >> >> Hi Ard, >> >> >> >> On Thu, Jul 28, 2016 at 04:15:14PM +0200, Ard Biesheuvel wrote: >> >>> Due to the untyped KIMAGE_VADDR constant, the linker may not notice >> >>> that the __rela_offset and __dynsym_offset expressions are absolute >> >>> values (i.e., are not subject to relocation). This does not matter for >> >>> KASLR, but it does confuse kallsyms in relative mode, since it uses >> >>> the lowest non-absolute symbol address as the anchor point, and expects >> >>> all other symbol addresses to be within 4 GB of it. >> >>> >> >>> Fix this by qualifying these expressions as ABSOLUTE() explicitly. >> >>> >> >>> Fixes: 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map") >> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >>> --- >> >>> arch/arm64/kernel/vmlinux.lds.S | 4 ++-- >> >>> 1 file changed, 2 insertions(+), 2 deletions(-) >> >>> >> >>> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S >> >>> index 89d6e177ecbd..d49492179967 100644 >> >>> --- a/arch/arm64/kernel/vmlinux.lds.S >> >>> +++ b/arch/arm64/kernel/vmlinux.lds.S >> >>> @@ -184,9 +184,9 @@ SECTIONS >> >>> *(.hash) >> >>> } >> >>> >> >>> - __rela_offset = ADDR(.rela) - KIMAGE_VADDR; >> >>> + __rela_offset = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR); >> >>> __rela_size = SIZEOF(.rela); >> >>> - __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR; >> >>> + __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR); >> >> >> >> You removed the __dynsym_offset entry entirely in "arm64: relocatable: >> >> suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part >> >> of the patch and assume you'll do a separate backport for stable. Ok? >> >> >> > >> > If you are ok with fixing up patches manually while applying, could we >> > just reorder them instead? >> >> Actually, this should go into v4.8-late not -stable > > Then the fixes tag is pretty confusing, since that's a commit from v4.7. > Sorry, my bad. I managed to confuse myself here. The tag is accurate, I was wrong. This should go into v4.7 as well. Note that the breakage I experienced was only with an LD built from binutils-gdb HEAD (2.28), so I guess there is no urgency here. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Fri, Jul 29, 2016 at 12:52:12PM +0200, Ard Biesheuvel wrote: > On 29 July 2016 at 12:49, Will Deacon <will.deacon@arm.com> wrote: > > On Fri, Jul 29, 2016 at 12:46:49PM +0200, Ard Biesheuvel wrote: > >> On 29 July 2016 at 11:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > >> > On 29 July 2016 at 11:06, Will Deacon <will.deacon@arm.com> wrote: > >> >> Hi Ard, > >> >> > >> >> On Thu, Jul 28, 2016 at 04:15:14PM +0200, Ard Biesheuvel wrote: > >> >>> Due to the untyped KIMAGE_VADDR constant, the linker may not notice > >> >>> that the __rela_offset and __dynsym_offset expressions are absolute > >> >>> values (i.e., are not subject to relocation). This does not matter for > >> >>> KASLR, but it does confuse kallsyms in relative mode, since it uses > >> >>> the lowest non-absolute symbol address as the anchor point, and expects > >> >>> all other symbol addresses to be within 4 GB of it. > >> >>> > >> >>> Fix this by qualifying these expressions as ABSOLUTE() explicitly. > >> >>> > >> >>> Fixes: 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map") > >> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> >>> --- > >> >>> arch/arm64/kernel/vmlinux.lds.S | 4 ++-- > >> >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >> >>> > >> >>> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > >> >>> index 89d6e177ecbd..d49492179967 100644 > >> >>> --- a/arch/arm64/kernel/vmlinux.lds.S > >> >>> +++ b/arch/arm64/kernel/vmlinux.lds.S > >> >>> @@ -184,9 +184,9 @@ SECTIONS > >> >>> *(.hash) > >> >>> } > >> >>> > >> >>> - __rela_offset = ADDR(.rela) - KIMAGE_VADDR; > >> >>> + __rela_offset = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR); > >> >>> __rela_size = SIZEOF(.rela); > >> >>> - __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR; > >> >>> + __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR); > >> >> > >> >> You removed the __dynsym_offset entry entirely in "arm64: relocatable: > >> >> suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part > >> >> of the patch and assume you'll do a separate backport for stable. Ok? > >> >> > >> > > >> > If you are ok with fixing up patches manually while applying, could we > >> > just reorder them instead? > >> > >> Actually, this should go into v4.8-late not -stable > > > > Then the fixes tag is pretty confusing, since that's a commit from v4.7. > > > > Sorry, my bad. I managed to confuse myself here. > The tag is accurate, I was wrong. This should go into v4.7 as well. Haha, ok! So if this is v4.7 material, what *should* the fixes tag be? > Note that the breakage I experienced was only with an LD built from > binutils-gdb HEAD (2.28), so I guess there is no urgency here. Sure, but people might upgrade their binutils before their kernel Image, so the backport is worthwhile. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 29 July 2016 at 12:57, Will Deacon <will.deacon@arm.com> wrote: > On Fri, Jul 29, 2016 at 12:52:12PM +0200, Ard Biesheuvel wrote: >> On 29 July 2016 at 12:49, Will Deacon <will.deacon@arm.com> wrote: >> > On Fri, Jul 29, 2016 at 12:46:49PM +0200, Ard Biesheuvel wrote: >> >> On 29 July 2016 at 11:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> >> > On 29 July 2016 at 11:06, Will Deacon <will.deacon@arm.com> wrote: >> >> >> Hi Ard, >> >> >> >> >> >> On Thu, Jul 28, 2016 at 04:15:14PM +0200, Ard Biesheuvel wrote: >> >> >>> Due to the untyped KIMAGE_VADDR constant, the linker may not notice >> >> >>> that the __rela_offset and __dynsym_offset expressions are absolute >> >> >>> values (i.e., are not subject to relocation). This does not matter for >> >> >>> KASLR, but it does confuse kallsyms in relative mode, since it uses >> >> >>> the lowest non-absolute symbol address as the anchor point, and expects >> >> >>> all other symbol addresses to be within 4 GB of it. >> >> >>> >> >> >>> Fix this by qualifying these expressions as ABSOLUTE() explicitly. >> >> >>> >> >> >>> Fixes: 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map") >> >> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> >>> --- >> >> >>> arch/arm64/kernel/vmlinux.lds.S | 4 ++-- >> >> >>> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> >>> >> >> >>> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S >> >> >>> index 89d6e177ecbd..d49492179967 100644 >> >> >>> --- a/arch/arm64/kernel/vmlinux.lds.S >> >> >>> +++ b/arch/arm64/kernel/vmlinux.lds.S >> >> >>> @@ -184,9 +184,9 @@ SECTIONS >> >> >>> *(.hash) >> >> >>> } >> >> >>> >> >> >>> - __rela_offset = ADDR(.rela) - KIMAGE_VADDR; >> >> >>> + __rela_offset = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR); >> >> >>> __rela_size = SIZEOF(.rela); >> >> >>> - __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR; >> >> >>> + __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR); >> >> >> >> >> >> You removed the __dynsym_offset entry entirely in "arm64: relocatable: >> >> >> suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part >> >> >> of the patch and assume you'll do a separate backport for stable. Ok? >> >> >> >> >> > >> >> > If you are ok with fixing up patches manually while applying, could we >> >> > just reorder them instead? >> >> >> >> Actually, this should go into v4.8-late not -stable >> > >> > Then the fixes tag is pretty confusing, since that's a commit from v4.7. >> > >> >> Sorry, my bad. I managed to confuse myself here. >> The tag is accurate, I was wrong. This should go into v4.7 as well. > > Haha, ok! So if this is v4.7 material, what *should* the fixes tag be? > As I said, the fixes tag is accurate. So it should still go into 4.8-late but with a cc:stable. The [tweaked] "suppress R_AARCH64_ABS64 relocations in vmlinux" could simply go on top, with no backport (it is just an optimization, not a bugfix) >> Note that the breakage I experienced was only with an LD built from >> binutils-gdb HEAD (2.28), so I guess there is no urgency here. > > Sure, but people might upgrade their binutils before their kernel Image, > so the backport is worthwhile. > That was my reasoning as well, but I was just highlighting that nothing is broken currently with released toolchains. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S index 89d6e177ecbd..d49492179967 100644 --- a/arch/arm64/kernel/vmlinux.lds.S +++ b/arch/arm64/kernel/vmlinux.lds.S @@ -184,9 +184,9 @@ SECTIONS *(.hash) } - __rela_offset = ADDR(.rela) - KIMAGE_VADDR; + __rela_offset = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR); __rela_size = SIZEOF(.rela); - __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR; + __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR); . = ALIGN(SEGMENT_ALIGN); __init_end = .;
Due to the untyped KIMAGE_VADDR constant, the linker may not notice that the __rela_offset and __dynsym_offset expressions are absolute values (i.e., are not subject to relocation). This does not matter for KASLR, but it does confuse kallsyms in relative mode, since it uses the lowest non-absolute symbol address as the anchor point, and expects all other symbol addresses to be within 4 GB of it. Fix this by qualifying these expressions as ABSOLUTE() explicitly. Fixes: 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map") Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/arm64/kernel/vmlinux.lds.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel