Clang warns:
arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
^
arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
extern u64 x86_spec_ctrl_current;
^
1 error generated.
The declaration should be using DECLARE_PER_CPU instead so all
attributes stay in sync.
Cc: [email protected]
Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---
v1 -> v2: https://lore.kernel.org/[email protected]/
* Use asm/percpu.h instead of linux/percpu.h to avoid static call
include errors.
arch/x86/include/asm/nospec-branch.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index bb05ed4f46bd..10a3bfc1eb23 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -11,6 +11,7 @@
#include <asm/cpufeatures.h>
#include <asm/msr-index.h>
#include <asm/unwind_hints.h>
+#include <asm/percpu.h>
#define RETPOLINE_THUNK_SIZE 32
@@ -280,7 +281,7 @@ static inline void indirect_branch_prediction_barrier(void)
/* The Intel SPEC CTRL MSR base value cache */
extern u64 x86_spec_ctrl_base;
-extern u64 x86_spec_ctrl_current;
+DECLARE_PER_CPU(u64, x86_spec_ctrl_current);
extern void write_spec_ctrl_current(u64 val, bool force);
extern u64 spec_ctrl_current(void);
base-commit: 72a8e05d4f66b5af7854df4490e3135168694b6b
--
2.37.1
On Wed, Jul 13, 2022 at 8:25 AM Nathan Chancellor <[email protected]> wrote:
>
> Clang warns:
>
> arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
> DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
> ^
> arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
> extern u64 x86_spec_ctrl_current;
> ^
> 1 error generated.
>
> The declaration should be using DECLARE_PER_CPU instead so all
> attributes stay in sync.
>
> Cc: [email protected]
> Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <[email protected]>
Tested-by: Nick Desaulniers <[email protected]>
> ---
>
> v1 -> v2: https://lore.kernel.org/[email protected]/
>
> * Use asm/percpu.h instead of linux/percpu.h to avoid static call
> include errors.
>
> arch/x86/include/asm/nospec-branch.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index bb05ed4f46bd..10a3bfc1eb23 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -11,6 +11,7 @@
> #include <asm/cpufeatures.h>
> #include <asm/msr-index.h>
> #include <asm/unwind_hints.h>
> +#include <asm/percpu.h>
>
> #define RETPOLINE_THUNK_SIZE 32
>
> @@ -280,7 +281,7 @@ static inline void indirect_branch_prediction_barrier(void)
>
> /* The Intel SPEC CTRL MSR base value cache */
> extern u64 x86_spec_ctrl_base;
> -extern u64 x86_spec_ctrl_current;
> +DECLARE_PER_CPU(u64, x86_spec_ctrl_current);
> extern void write_spec_ctrl_current(u64 val, bool force);
> extern u64 spec_ctrl_current(void);
>
>
> base-commit: 72a8e05d4f66b5af7854df4490e3135168694b6b
> --
> 2.37.1
>
>
--
Thanks,
~Nick Desaulniers
On Wed, Jul 13, 2022 at 08:24:37AM -0700, Nathan Chancellor wrote:
> Clang warns:
>
> arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
> DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
> ^
> arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
> extern u64 x86_spec_ctrl_current;
> ^
> 1 error generated.
>
> The declaration should be using DECLARE_PER_CPU instead so all
> attributes stay in sync.
>
> Cc: [email protected]
> Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
>
> v1 -> v2: https://lore.kernel.org/[email protected]/
>
> * Use asm/percpu.h instead of linux/percpu.h to avoid static call
> include errors.
>
> arch/x86/include/asm/nospec-branch.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index bb05ed4f46bd..10a3bfc1eb23 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -11,6 +11,7 @@
> #include <asm/cpufeatures.h>
> #include <asm/msr-index.h>
> #include <asm/unwind_hints.h>
> +#include <asm/percpu.h>
>
> #define RETPOLINE_THUNK_SIZE 32
>
When I tried this earlier today I ran into cyclic headers, you sure this
works?
On Wed, Jul 13, 2022 at 06:22:41PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 13, 2022 at 08:24:37AM -0700, Nathan Chancellor wrote:
> > Clang warns:
> >
> > arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
> > DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
> > ^
> > arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
> > extern u64 x86_spec_ctrl_current;
> > ^
> > 1 error generated.
> >
> > The declaration should be using DECLARE_PER_CPU instead so all
> > attributes stay in sync.
> >
> > Cc: [email protected]
> > Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
> > Reported-by: kernel test robot <[email protected]>
> > Signed-off-by: Nathan Chancellor <[email protected]>
> > ---
> >
> > v1 -> v2: https://lore.kernel.org/[email protected]/
> >
> > * Use asm/percpu.h instead of linux/percpu.h to avoid static call
> > include errors.
> >
> > arch/x86/include/asm/nospec-branch.h | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> > index bb05ed4f46bd..10a3bfc1eb23 100644
> > --- a/arch/x86/include/asm/nospec-branch.h
> > +++ b/arch/x86/include/asm/nospec-branch.h
> > @@ -11,6 +11,7 @@
> > #include <asm/cpufeatures.h>
> > #include <asm/msr-index.h>
> > #include <asm/unwind_hints.h>
> > +#include <asm/percpu.h>
> >
> > #define RETPOLINE_THUNK_SIZE 32
> >
>
> When I tried this earlier today I ran into cyclic headers, you sure this
> works?
Yes, I did my regular set of x86 builds with clang and they all passed.
Cheers,
Nathan
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: db866d3352ec85e821e730e191481cba3a2bfa6e
Gitweb: https://git.kernel.org/tip/db866d3352ec85e821e730e191481cba3a2bfa6e
Author: Nathan Chancellor <[email protected]>
AuthorDate: Wed, 13 Jul 2022 08:24:37 -07:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Thu, 14 Jul 2022 13:40:21 +02:00
x86/speculation: Use DECLARE_PER_CPU for x86_spec_ctrl_current
Clang warns:
arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
^
arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
extern u64 x86_spec_ctrl_current;
^
1 error generated.
The declaration should be using DECLARE_PER_CPU instead so all
attributes stay in sync.
Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Tested-by: Nick Desaulniers <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/nospec-branch.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index bb05ed4..10a3bfc 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -11,6 +11,7 @@
#include <asm/cpufeatures.h>
#include <asm/msr-index.h>
#include <asm/unwind_hints.h>
+#include <asm/percpu.h>
#define RETPOLINE_THUNK_SIZE 32
@@ -280,7 +281,7 @@ static inline void indirect_branch_prediction_barrier(void)
/* The Intel SPEC CTRL MSR base value cache */
extern u64 x86_spec_ctrl_base;
-extern u64 x86_spec_ctrl_current;
+DECLARE_PER_CPU(u64, x86_spec_ctrl_current);
extern void write_spec_ctrl_current(u64 val, bool force);
extern u64 spec_ctrl_current(void);
On Wed, 13 Jul 2022 08:24:37 -0700 Nathan Chancellor wrote:
> Clang warns:
>
> arch/x86/kernel/cpu/bugs.c:58:21: error: section attribute is specified on redeclared variable [-Werror,-Wsection]
> DEFINE_PER_CPU(u64, x86_spec_ctrl_current);
> ^
> arch/x86/include/asm/nospec-branch.h:283:12: note: previous declaration is here
> extern u64 x86_spec_ctrl_current;
> ^
> 1 error generated.
>
> The declaration should be using DECLARE_PER_CPU instead so all
> attributes stay in sync.
>
> Cc: [email protected]
> Fixes: fc02735b14ff ("KVM: VMX: Prevent guest RSB poisoning attacks with eIBRS")
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
Hi, sorry to bother, any idea on the ETA for this fix getting into
Linus's tree? I'm trying to figure out if we should wait with
forwarding the networking trees or this will take a while.
On Thu, 14 Jul 2022 14:51:52 -0700 Linus Torvalds wrote:
> On Thu, Jul 14, 2022 at 2:30 PM Jakub Kicinski <[email protected]> wrote:
> > Hi, sorry to bother, any idea on the ETA for this fix getting into
> > Linus's tree? I'm trying to figure out if we should wait with
> > forwarding the networking trees or this will take a while.
>
> Well, I have that patch by now in my own clang tree, but was holding
> it off just because I was expecting a few other fixes for the fallout.
>
> But if this particular one causes problems for maintainers, I can
> easily just take it right away just cherry-pick it from my own
> test-tree to my "main" tree.
I have clang 13, let me double check this fix is enough for the build
to complete without disabling WERROR. If it's not a hassle it'd
certainly make my life easier if the fix gotten applied now...
On Thu, Jul 14, 2022 at 2:51 PM Linus Torvalds
<[email protected]> wrote:
>
> But if this particular one causes problems for maintainers, I can
> easily just take it right away just cherry-pick it from my own
> test-tree to my "main" tree.
Oh, and as I did that cherry-pick, I suddenly remembered that I think
PeterZ had a slightly different version of it - the one I picked up is
Nathan's "v2".
PeterZ, do you have preferences? I've waited this long, I might as
well wait a bit more before I push out whatever version people prefer.
Linus
On Thu, Jul 14, 2022 at 2:30 PM Jakub Kicinski <[email protected]> wrote:
>
> Hi, sorry to bother, any idea on the ETA for this fix getting into
> Linus's tree? I'm trying to figure out if we should wait with
> forwarding the networking trees or this will take a while.
Well, I have that patch by now in my own clang tree, but was holding
it off just because I was expecting a few other fixes for the fallout.
But if this particular one causes problems for maintainers, I can
easily just take it right away just cherry-pick it from my own
test-tree to my "main" tree.
Linus
On Thu, 14 Jul 2022 15:14:53 -0700 Linus Torvalds wrote:
> On Thu, Jul 14, 2022 at 2:56 PM Jakub Kicinski <[email protected]> wrote:
> > I have clang 13, let me double check this fix is enough for the build
> > to complete without disabling WERROR.
>
> I have clang 14 locally, and it builds fine with that (and doesn't
> build without it).
FWIW I can confirm - builds with clang 13 as well.
> I actually normally build the kernel with both gcc and clang. My
> "upstream" kernel I build with gcc, and then I have my "private random
> collection of patches" kernel that I build with clang and that are
> just rebased on top of the kernel-of-the-day.
>
> This is all entirely for historical reasons - part of my "private
> random collection of patches" used to be the "asm goto with outputs",
> which had clang support first.
>
> But then the reason I never even noticed the build breakage with the
> retbleed patches until much too late was that those I just had as a
> third fork off my upstream kernel, so despite me usually building with
> clang too, that only got attention from gcc.
>
> So it's really just a microcosm version of the exact same bigger issue
> we always have with those embargoed hw security patches: they end up
> missing out on all the usual test environments.
>
> Anyway, I cherry-picked Nathan's patch from my clang tree and pushed
> it out as commit db886979683a ("x86/speculation: Use DECLARE_PER_CPU
> for x86_spec_ctrl_current").
Awesome, thanks!
On Thu, Jul 14, 2022 at 02:56:12PM -0700, Linus Torvalds wrote:
> On Thu, Jul 14, 2022 at 2:51 PM Linus Torvalds
> <[email protected]> wrote:
> >
> > But if this particular one causes problems for maintainers, I can
> > easily just take it right away just cherry-pick it from my own
> > test-tree to my "main" tree.
>
> Oh, and as I did that cherry-pick, I suddenly remembered that I think
> PeterZ had a slightly different version of it - the one I picked up is
> Nathan's "v2".
>
> PeterZ, do you have preferences? I've waited this long, I might as
> well wait a bit more before I push out whatever version people prefer.
Nathan's is much nicer; I got bit by header hell and punted.
On Thu, Jul 14, 2022 at 2:56 PM Jakub Kicinski <[email protected]> wrote:
>
> I have clang 13, let me double check this fix is enough for the build
> to complete without disabling WERROR.
I have clang 14 locally, and it builds fine with that (and doesn't
build without it).
I actually normally build the kernel with both gcc and clang. My
"upstream" kernel I build with gcc, and then I have my "private random
collection of patches" kernel that I build with clang and that are
just rebased on top of the kernel-of-the-day.
This is all entirely for historical reasons - part of my "private
random collection of patches" used to be the "asm goto with outputs",
which had clang support first.
But then the reason I never even noticed the build breakage with the
retbleed patches until much too late was that those I just had as a
third fork off my upstream kernel, so despite me usually building with
clang too, that only got attention from gcc.
So it's really just a microcosm version of the exact same bigger issue
we always have with those embargoed hw security patches: they end up
missing out on all the usual test environments.
Anyway, I cherry-picked Nathan's patch from my clang tree and pushed
it out as commit db886979683a ("x86/speculation: Use DECLARE_PER_CPU
for x86_spec_ctrl_current").
Linus
On Thu, Jul 14, 2022 at 03:14:53PM -0700, Linus Torvalds wrote:
> Anyway, I cherry-picked Nathan's patch from my clang tree and pushed
> it out as commit db886979683a ("x86/speculation: Use DECLARE_PER_CPU
> for x86_spec_ctrl_current").
... and I've zapped it from my lineup.
Thx.
--
Regards/Gruss,
Boris.
SUSE Software Solutions Germany GmbH
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
(HRB 36809, AG Nürnberg)