* [PATCH] arm64/vdso: Remove unused makefile variable @ 2020-04-24 15:58 Geoff Levand 2020-04-27 20:35 ` Will Deacon 0 siblings, 1 reply; 8+ messages in thread From: Geoff Levand @ 2020-04-24 15:58 UTC (permalink / raw) To: Catalin Marinas; +Cc: Will Deacon, linux-arm-kernel The vdso makefile variable VDSO_LDFLAGS is defined, but never used, so remove it. Signed-off-by: Geoff Levand <geoff@infradead.org> --- Hi, This seems to be left over from a code cleanup that missed it. -Geoff arch/arm64/kernel/vdso/Makefile | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile index dd2514bb1511..6f6b55c12029 100644 --- a/arch/arm64/kernel/vdso/Makefile +++ b/arch/arm64/kernel/vdso/Makefile @@ -23,8 +23,6 @@ ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 --hash-style=sysv \ ccflags-y := -fno-common -fno-builtin -fno-stack-protector -ffixed-x18 ccflags-y += -DDISABLE_BRANCH_PROFILING -VDSO_LDFLAGS := -Bsymbolic - CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os KBUILD_CFLAGS += $(DISABLE_LTO) KASAN_SANITIZE := n -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64/vdso: Remove unused makefile variable 2020-04-24 15:58 [PATCH] arm64/vdso: Remove unused makefile variable Geoff Levand @ 2020-04-27 20:35 ` Will Deacon 2020-04-28 10:46 ` Vincenzo Frascino 0 siblings, 1 reply; 8+ messages in thread From: Will Deacon @ 2020-04-27 20:35 UTC (permalink / raw) To: Geoff Levand; +Cc: Catalin Marinas, vincenzo.frascino, linux-arm-kernel [+Vincenzo] On Fri, Apr 24, 2020 at 08:58:49AM -0700, Geoff Levand wrote: > The vdso makefile variable VDSO_LDFLAGS is defined, but never used, > so remove it. > > Signed-off-by: Geoff Levand <geoff@infradead.org> > --- > > Hi, > > This seems to be left over from a code cleanup that missed it. While I agree that this isn't used, I'm wondering why '-Bsymbolic' is used to link the compat vDSO but not the native one. It seems weird to differ in this regard. Vincenzo? Looks like you added this unused variable in 28b1a824a4f44 ("arm64: vdso: Substitute gettimeofday() with C implementation"). Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64/vdso: Remove unused makefile variable 2020-04-27 20:35 ` Will Deacon @ 2020-04-28 10:46 ` Vincenzo Frascino 2020-04-28 11:52 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Vincenzo Frascino @ 2020-04-28 10:46 UTC (permalink / raw) To: Will Deacon, Geoff Levand; +Cc: Catalin Marinas, linux-arm-kernel Hi Will, thank you for pointing this out. On 4/27/20 9:35 PM, Will Deacon wrote: > [+Vincenzo] > > On Fri, Apr 24, 2020 at 08:58:49AM -0700, Geoff Levand wrote: >> The vdso makefile variable VDSO_LDFLAGS is defined, but never used, >> so remove it. >> >> Signed-off-by: Geoff Levand <geoff@infradead.org> >> --- >> >> Hi, >> >> This seems to be left over from a code cleanup that missed it. > > While I agree that this isn't used, I'm wondering why '-Bsymbolic' is used > to link the compat vDSO but not the native one. It seems weird to differ > in this regard. > > Vincenzo? Looks like you added this unused variable in 28b1a824a4f44 > ("arm64: vdso: Substitute gettimeofday() with C implementation"). > My understanding is that "-Bsymbolic" is required by both compat and normal vdso because when the shared library is built it adds a flag in the dynamic section of the binary called DT_SYMBOLIC which alters the dynamic linker's symbol resolution algorithm to search for references for a symbol inside the library first and then into the executable. This becomes useful for example when an executable built with -fPIC is trying to call a public vDSO function from assembly (bl symbol). The issue here seems to be that I used VDSO_LDFLAGS instead of ldflags-y. I can post a patch and Cc stable. Adding Geoff as Reported-by. > Will > -- Regards, Vincenzo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64/vdso: Remove unused makefile variable 2020-04-28 10:46 ` Vincenzo Frascino @ 2020-04-28 11:52 ` Ard Biesheuvel 2020-04-28 12:43 ` Will Deacon 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2020-04-28 11:52 UTC (permalink / raw) To: Vincenzo Frascino Cc: Geoff Levand, Catalin Marinas, Will Deacon, linux-arm-kernel On Tue, 28 Apr 2020 at 12:45, Vincenzo Frascino <vincenzo.frascino@arm.com> wrote: > > Hi Will, > > thank you for pointing this out. > > On 4/27/20 9:35 PM, Will Deacon wrote: > > [+Vincenzo] > > > > On Fri, Apr 24, 2020 at 08:58:49AM -0700, Geoff Levand wrote: > >> The vdso makefile variable VDSO_LDFLAGS is defined, but never used, > >> so remove it. > >> > >> Signed-off-by: Geoff Levand <geoff@infradead.org> > >> --- > >> > >> Hi, > >> > >> This seems to be left over from a code cleanup that missed it. > > > > While I agree that this isn't used, I'm wondering why '-Bsymbolic' is used > > to link the compat vDSO but not the native one. It seems weird to differ > > in this regard. > > > > Vincenzo? Looks like you added this unused variable in 28b1a824a4f44 > > ("arm64: vdso: Substitute gettimeofday() with C implementation"). > > > > My understanding is that "-Bsymbolic" is required by both compat and normal vdso > because when the shared library is built it adds a flag in the dynamic section > of the binary called DT_SYMBOLIC which alters the dynamic linker's symbol > resolution algorithm to search for references for a symbol inside the library > first and then into the executable. > DT_SYMBOLIC doesn't (or shouldn't) change the dynamic linking behavior. It informs the linker that ELF symbol preemption may not work, since the .so has bound internal references to its exported symbols to the internal versions directly, rather than allowing the application to supersede (i.e., 'preempt') them. This is an obscure feature that isn't really relevant for the VDSO, since we carefully control what we export from the .so anyway (via the linker script's VERSIONS section) > This becomes useful for example when an executable built with -fPIC is trying to > call a public vDSO function from assembly (bl symbol). > > The issue here seems to be that I used VDSO_LDFLAGS instead of ldflags-y. I can > post a patch and Cc stable. Adding Geoff as Reported-by. > I think it can be removed. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64/vdso: Remove unused makefile variable 2020-04-28 11:52 ` Ard Biesheuvel @ 2020-04-28 12:43 ` Will Deacon 2020-04-28 13:02 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Will Deacon @ 2020-04-28 12:43 UTC (permalink / raw) To: Ard Biesheuvel Cc: Geoff Levand, Catalin Marinas, Vincenzo Frascino, linux-arm-kernel On Tue, Apr 28, 2020 at 01:52:55PM +0200, Ard Biesheuvel wrote: > On Tue, 28 Apr 2020 at 12:45, Vincenzo Frascino > <vincenzo.frascino@arm.com> wrote: > > On 4/27/20 9:35 PM, Will Deacon wrote: > > > [+Vincenzo] > > > > > > On Fri, Apr 24, 2020 at 08:58:49AM -0700, Geoff Levand wrote: > > >> The vdso makefile variable VDSO_LDFLAGS is defined, but never used, > > >> so remove it. > > >> > > >> Signed-off-by: Geoff Levand <geoff@infradead.org> > > >> --- > > >> > > >> Hi, > > >> > > >> This seems to be left over from a code cleanup that missed it. > > > > > > While I agree that this isn't used, I'm wondering why '-Bsymbolic' is used > > > to link the compat vDSO but not the native one. It seems weird to differ > > > in this regard. > > > > > > Vincenzo? Looks like you added this unused variable in 28b1a824a4f44 > > > ("arm64: vdso: Substitute gettimeofday() with C implementation"). > > > > > > > My understanding is that "-Bsymbolic" is required by both compat and normal vdso > > because when the shared library is built it adds a flag in the dynamic section > > of the binary called DT_SYMBOLIC which alters the dynamic linker's symbol > > resolution algorithm to search for references for a symbol inside the library > > first and then into the executable. > > > > DT_SYMBOLIC doesn't (or shouldn't) change the dynamic linking > behavior. It informs the linker that ELF symbol preemption may not > work, since the .so has bound internal references to its exported > symbols to the internal versions directly, rather than allowing the > application to supersede (i.e., 'preempt') them. This is an obscure > feature that isn't really relevant for the VDSO, since we carefully > control what we export from the .so anyway (via the linker script's > VERSIONS section) > > > This becomes useful for example when an executable built with -fPIC is trying to > > call a public vDSO function from assembly (bl symbol). > > > > The issue here seems to be that I used VDSO_LDFLAGS instead of ldflags-y. I can > > post a patch and Cc stable. Adding Geoff as Reported-by. > > > > I think it can be removed. Hmm, so I did a little bit more digging because -Bsymbolic is used to link the vDSO on arm, mips, sparc and x86. Commit 6f121e548f83 ("x86, vdso: Reimplement vdso.so preparation in build-time C") suggests that it's a good idea to prevent any unexpected dynamic relocations appearing in the vDSO object. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64/vdso: Remove unused makefile variable 2020-04-28 12:43 ` Will Deacon @ 2020-04-28 13:02 ` Ard Biesheuvel 2020-04-28 13:16 ` Will Deacon 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2020-04-28 13:02 UTC (permalink / raw) To: Will Deacon Cc: Geoff Levand, Catalin Marinas, Vincenzo Frascino, linux-arm-kernel On Tue, 28 Apr 2020 at 14:43, Will Deacon <will@kernel.org> wrote: > > On Tue, Apr 28, 2020 at 01:52:55PM +0200, Ard Biesheuvel wrote: > > On Tue, 28 Apr 2020 at 12:45, Vincenzo Frascino > > <vincenzo.frascino@arm.com> wrote: > > > On 4/27/20 9:35 PM, Will Deacon wrote: > > > > [+Vincenzo] > > > > > > > > On Fri, Apr 24, 2020 at 08:58:49AM -0700, Geoff Levand wrote: > > > >> The vdso makefile variable VDSO_LDFLAGS is defined, but never used, > > > >> so remove it. > > > >> > > > >> Signed-off-by: Geoff Levand <geoff@infradead.org> > > > >> --- > > > >> > > > >> Hi, > > > >> > > > >> This seems to be left over from a code cleanup that missed it. > > > > > > > > While I agree that this isn't used, I'm wondering why '-Bsymbolic' is used > > > > to link the compat vDSO but not the native one. It seems weird to differ > > > > in this regard. > > > > > > > > Vincenzo? Looks like you added this unused variable in 28b1a824a4f44 > > > > ("arm64: vdso: Substitute gettimeofday() with C implementation"). > > > > > > > > > > My understanding is that "-Bsymbolic" is required by both compat and normal vdso > > > because when the shared library is built it adds a flag in the dynamic section > > > of the binary called DT_SYMBOLIC which alters the dynamic linker's symbol > > > resolution algorithm to search for references for a symbol inside the library > > > first and then into the executable. > > > > > > > DT_SYMBOLIC doesn't (or shouldn't) change the dynamic linking > > behavior. It informs the linker that ELF symbol preemption may not > > work, since the .so has bound internal references to its exported > > symbols to the internal versions directly, rather than allowing the > > application to supersede (i.e., 'preempt') them. This is an obscure > > feature that isn't really relevant for the VDSO, since we carefully > > control what we export from the .so anyway (via the linker script's > > VERSIONS section) > > > > > This becomes useful for example when an executable built with -fPIC is trying to > > > call a public vDSO function from assembly (bl symbol). > > > > > > The issue here seems to be that I used VDSO_LDFLAGS instead of ldflags-y. I can > > > post a patch and Cc stable. Adding Geoff as Reported-by. > > > > > > > I think it can be removed. > > Hmm, so I did a little bit more digging because -Bsymbolic is used to link > the vDSO on arm, mips, sparc and x86. Commit 6f121e548f83 ("x86, vdso: > Reimplement vdso.so preparation in build-time C") suggests that it's a good > idea to prevent any unexpected dynamic relocations appearing in the vDSO > object. > In the x86 case, there are internal calls to the exported routines, and without Bsymbolic, those are routed via a GOT/PLT so that the application can override those symbols. For instance, under the normal ELF symbol preemption rules, the x86 VDSO should use the application's version of __kernel_vsyscall() if it exists as a global symbol, and so the interposable dynamic relocation is made to point to the application's version of the symbol. That is why x86 needs -Bsymbolic. That issue does not exist on arm64, as far as I can tell. It doesn't really hurt either to have the option, but it would be good to perhaps annotate why we are keeping it. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64/vdso: Remove unused makefile variable 2020-04-28 13:02 ` Ard Biesheuvel @ 2020-04-28 13:16 ` Will Deacon 2020-04-28 13:21 ` Vincenzo Frascino 0 siblings, 1 reply; 8+ messages in thread From: Will Deacon @ 2020-04-28 13:16 UTC (permalink / raw) To: Ard Biesheuvel Cc: Geoff Levand, Catalin Marinas, Vincenzo Frascino, linux-arm-kernel On Tue, Apr 28, 2020 at 03:02:43PM +0200, Ard Biesheuvel wrote: > On Tue, 28 Apr 2020 at 14:43, Will Deacon <will@kernel.org> wrote: > > > > On Tue, Apr 28, 2020 at 01:52:55PM +0200, Ard Biesheuvel wrote: > > > On Tue, 28 Apr 2020 at 12:45, Vincenzo Frascino > > > <vincenzo.frascino@arm.com> wrote: > > > > On 4/27/20 9:35 PM, Will Deacon wrote: > > > > > [+Vincenzo] > > > > > > > > > > On Fri, Apr 24, 2020 at 08:58:49AM -0700, Geoff Levand wrote: > > > > >> The vdso makefile variable VDSO_LDFLAGS is defined, but never used, > > > > >> so remove it. > > > > >> > > > > >> Signed-off-by: Geoff Levand <geoff@infradead.org> > > > > >> --- > > > > >> > > > > >> Hi, > > > > >> > > > > >> This seems to be left over from a code cleanup that missed it. > > > > > > > > > > While I agree that this isn't used, I'm wondering why '-Bsymbolic' is used > > > > > to link the compat vDSO but not the native one. It seems weird to differ > > > > > in this regard. > > > > > > > > > > Vincenzo? Looks like you added this unused variable in 28b1a824a4f44 > > > > > ("arm64: vdso: Substitute gettimeofday() with C implementation"). > > > > > > > > > > > > > My understanding is that "-Bsymbolic" is required by both compat and normal vdso > > > > because when the shared library is built it adds a flag in the dynamic section > > > > of the binary called DT_SYMBOLIC which alters the dynamic linker's symbol > > > > resolution algorithm to search for references for a symbol inside the library > > > > first and then into the executable. > > > > > > > > > > DT_SYMBOLIC doesn't (or shouldn't) change the dynamic linking > > > behavior. It informs the linker that ELF symbol preemption may not > > > work, since the .so has bound internal references to its exported > > > symbols to the internal versions directly, rather than allowing the > > > application to supersede (i.e., 'preempt') them. This is an obscure > > > feature that isn't really relevant for the VDSO, since we carefully > > > control what we export from the .so anyway (via the linker script's > > > VERSIONS section) > > > > > > > This becomes useful for example when an executable built with -fPIC is trying to > > > > call a public vDSO function from assembly (bl symbol). > > > > > > > > The issue here seems to be that I used VDSO_LDFLAGS instead of ldflags-y. I can > > > > post a patch and Cc stable. Adding Geoff as Reported-by. > > > > > > > > > > I think it can be removed. > > > > Hmm, so I did a little bit more digging because -Bsymbolic is used to link > > the vDSO on arm, mips, sparc and x86. Commit 6f121e548f83 ("x86, vdso: > > Reimplement vdso.so preparation in build-time C") suggests that it's a good > > idea to prevent any unexpected dynamic relocations appearing in the vDSO > > object. > > > > In the x86 case, there are internal calls to the exported routines, > and without Bsymbolic, those are routed via a GOT/PLT so that the > application can override those symbols. For instance, under the normal > ELF symbol preemption rules, the x86 VDSO should use the application's > version of __kernel_vsyscall() if it exists as a global symbol, and so > the interposable dynamic relocation is made to point to the > application's version of the symbol. That is why x86 needs -Bsymbolic. > > That issue does not exist on arm64, as far as I can tell. It doesn't > really hurt either to have the option, but it would be good to perhaps > annotate why we are keeping it. Yes, so I think we either remove it for arm, arm64 compat and arm64 native or we add it to arm64 native for consistency/over-zealous future-proofing. In either case, we need to document it somewhere so we don't run into this again in future. Vincenzo -- are you able to send a patch, please? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64/vdso: Remove unused makefile variable 2020-04-28 13:16 ` Will Deacon @ 2020-04-28 13:21 ` Vincenzo Frascino 0 siblings, 0 replies; 8+ messages in thread From: Vincenzo Frascino @ 2020-04-28 13:21 UTC (permalink / raw) To: Will Deacon, Ard Biesheuvel Cc: Geoff Levand, Catalin Marinas, linux-arm-kernel Hi Will, On 4/28/20 2:16 PM, Will Deacon wrote: > On Tue, Apr 28, 2020 at 03:02:43PM +0200, Ard Biesheuvel wrote: >> On Tue, 28 Apr 2020 at 14:43, Will Deacon <will@kernel.org> wrote: >>> >>> On Tue, Apr 28, 2020 at 01:52:55PM +0200, Ard Biesheuvel wrote: >>>> On Tue, 28 Apr 2020 at 12:45, Vincenzo Frascino >>>> <vincenzo.frascino@arm.com> wrote: >>>>> On 4/27/20 9:35 PM, Will Deacon wrote: >>>>>> [+Vincenzo] >>>>>> >>>>>> On Fri, Apr 24, 2020 at 08:58:49AM -0700, Geoff Levand wrote: >>>>>>> The vdso makefile variable VDSO_LDFLAGS is defined, but never used, >>>>>>> so remove it. >>>>>>> >>>>>>> Signed-off-by: Geoff Levand <geoff@infradead.org> >>>>>>> --- >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> This seems to be left over from a code cleanup that missed it. >>>>>> >>>>>> While I agree that this isn't used, I'm wondering why '-Bsymbolic' is used >>>>>> to link the compat vDSO but not the native one. It seems weird to differ >>>>>> in this regard. >>>>>> >>>>>> Vincenzo? Looks like you added this unused variable in 28b1a824a4f44 >>>>>> ("arm64: vdso: Substitute gettimeofday() with C implementation"). >>>>>> >>>>> >>>>> My understanding is that "-Bsymbolic" is required by both compat and normal vdso >>>>> because when the shared library is built it adds a flag in the dynamic section >>>>> of the binary called DT_SYMBOLIC which alters the dynamic linker's symbol >>>>> resolution algorithm to search for references for a symbol inside the library >>>>> first and then into the executable. >>>>> >>>> >>>> DT_SYMBOLIC doesn't (or shouldn't) change the dynamic linking >>>> behavior. It informs the linker that ELF symbol preemption may not >>>> work, since the .so has bound internal references to its exported >>>> symbols to the internal versions directly, rather than allowing the >>>> application to supersede (i.e., 'preempt') them. This is an obscure >>>> feature that isn't really relevant for the VDSO, since we carefully >>>> control what we export from the .so anyway (via the linker script's >>>> VERSIONS section) >>>> >>>>> This becomes useful for example when an executable built with -fPIC is trying to >>>>> call a public vDSO function from assembly (bl symbol). >>>>> >>>>> The issue here seems to be that I used VDSO_LDFLAGS instead of ldflags-y. I can >>>>> post a patch and Cc stable. Adding Geoff as Reported-by. >>>>> >>>> >>>> I think it can be removed. >>> >>> Hmm, so I did a little bit more digging because -Bsymbolic is used to link >>> the vDSO on arm, mips, sparc and x86. Commit 6f121e548f83 ("x86, vdso: >>> Reimplement vdso.so preparation in build-time C") suggests that it's a good >>> idea to prevent any unexpected dynamic relocations appearing in the vDSO >>> object. >>> >> >> In the x86 case, there are internal calls to the exported routines, >> and without Bsymbolic, those are routed via a GOT/PLT so that the >> application can override those symbols. For instance, under the normal >> ELF symbol preemption rules, the x86 VDSO should use the application's >> version of __kernel_vsyscall() if it exists as a global symbol, and so >> the interposable dynamic relocation is made to point to the >> application's version of the symbol. That is why x86 needs -Bsymbolic. >> >> That issue does not exist on arm64, as far as I can tell. It doesn't >> really hurt either to have the option, but it would be good to perhaps >> annotate why we are keeping it. > > Yes, so I think we either remove it for arm, arm64 compat and arm64 native > or we add it to arm64 native for consistency/over-zealous future-proofing. > In either case, we need to document it somewhere so we don't run into this > again in future. > > Vincenzo -- are you able to send a patch, please? > Already working on it. My preference is to keep it, I am adding documentation to the patch notes and in the Makefile. > Will > -- Regards, Vincenzo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-04-28 13:26 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-24 15:58 [PATCH] arm64/vdso: Remove unused makefile variable Geoff Levand 2020-04-27 20:35 ` Will Deacon 2020-04-28 10:46 ` Vincenzo Frascino 2020-04-28 11:52 ` Ard Biesheuvel 2020-04-28 12:43 ` Will Deacon 2020-04-28 13:02 ` Ard Biesheuvel 2020-04-28 13:16 ` Will Deacon 2020-04-28 13:21 ` Vincenzo Frascino
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).