linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value
@ 2015-04-03  9:42 Borislav Petkov
  2015-04-03 11:20 ` Borislav Petkov
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Borislav Petkov @ 2015-04-03  9:42 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Denys Vlasenko, Andy Lutomirski, Alexei Starovoitov,
	Frederic Weisbecker, H. Peter Anvin, Kees Cook, Linus Torvalds,
	Oleg Nesterov, Steven Rostedt, Will Drewry

From: Borislav Petkov <bp@suse.de>

Commit

  d56fe4bf5f3c ("x86/asm/entry/64: Always set up SYSENTER MSRs")

missed to add "ULL" to the 0 and wrmsrl_safe() complains:

  arch/x86/kernel/cpu/common.c: In function ‘syscall_init’:
  arch/x86/kernel/cpu/common.c:1226:2: warning: right shift count >= width of type
    wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0);

Fix it.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index bb6633a87a19..2c249b57e4b5 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1223,7 +1223,7 @@ void syscall_init(void)
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)ia32_sysenter_target);
 #else
 	wrmsrl(MSR_CSTAR, ignore_sysret);
-	wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0);
+	wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0ULL);
 	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, 0ULL);
 #endif
-- 
2.3.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value
  2015-04-03  9:42 [PATCH] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value Borislav Petkov
@ 2015-04-03 11:20 ` Borislav Petkov
  2015-04-03 11:52   ` Denys Vlasenko
  2015-04-03 14:07 ` [tip:x86/asm] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value tip-bot for Borislav Petkov
  2015-04-03 14:07 ` [tip:x86/asm] x86/asm/entry/64: Use a define for an invalid segment selector tip-bot for Borislav Petkov
  2 siblings, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2015-04-03 11:20 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Denys Vlasenko, Andy Lutomirski, Alexei Starovoitov,
	Frederic Weisbecker, H. Peter Anvin, Kees Cook, Linus Torvalds,
	Oleg Nesterov, Steven Rostedt, Will Drewry

On Fri, Apr 03, 2015 at 11:42:10AM +0200, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> Commit
> 
>   d56fe4bf5f3c ("x86/asm/entry/64: Always set up SYSENTER MSRs")
> 
> missed to add "ULL" to the 0 and wrmsrl_safe() complains:
> 
>   arch/x86/kernel/cpu/common.c: In function ‘syscall_init’:
>   arch/x86/kernel/cpu/common.c:1226:2: warning: right shift count >= width of type
>     wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0);
> 
> Fix it.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Denys Vlasenko <dvlasenk@redhat.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Alexei Starovoitov <ast@plumgrid.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Will Drewry <wad@chromium.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/kernel/cpu/common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index bb6633a87a19..2c249b57e4b5 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1223,7 +1223,7 @@ void syscall_init(void)
>  	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)ia32_sysenter_target);
>  #else
>  	wrmsrl(MSR_CSTAR, ignore_sysret);
> -	wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0);
> +	wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0ULL);

Btw, we probably should define an invalid segment

#define GDT_ENTRY_INVALID_SEGMENT		0ULL

and use that value instead of the naked 0.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value
  2015-04-03 11:20 ` Borislav Petkov
@ 2015-04-03 11:52   ` Denys Vlasenko
  2015-04-03 12:05     ` Borislav Petkov
  0 siblings, 1 reply; 7+ messages in thread
From: Denys Vlasenko @ 2015-04-03 11:52 UTC (permalink / raw)
  To: Borislav Petkov, Ingo Molnar
  Cc: LKML, Andy Lutomirski, Alexei Starovoitov, Frederic Weisbecker,
	H. Peter Anvin, Kees Cook, Linus Torvalds, Oleg Nesterov,
	Steven Rostedt, Will Drewry

On 04/03/2015 01:20 PM, Borislav Petkov wrote:
> On Fri, Apr 03, 2015 at 11:42:10AM +0200, Borislav Petkov wrote:
>> From: Borislav Petkov <bp@suse.de>
>>
>> Commit
>>
>>   d56fe4bf5f3c ("x86/asm/entry/64: Always set up SYSENTER MSRs")
>>
>> missed to add "ULL" to the 0 and wrmsrl_safe() complains:
>>
>>   arch/x86/kernel/cpu/common.c: In function ‘syscall_init’:
>>   arch/x86/kernel/cpu/common.c:1226:2: warning: right shift count >= width of type
>>     wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0);
>>
>> Fix it.
>>
>> Signed-off-by: Borislav Petkov <bp@suse.de>
>> Cc: Denys Vlasenko <dvlasenk@redhat.com>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Alexei Starovoitov <ast@plumgrid.com>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Cc: H. Peter Anvin <hpa@zytor.com>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Oleg Nesterov <oleg@redhat.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Will Drewry <wad@chromium.org>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> ---
>>  arch/x86/kernel/cpu/common.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index bb6633a87a19..2c249b57e4b5 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -1223,7 +1223,7 @@ void syscall_init(void)
>>  	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)ia32_sysenter_target);
>>  #else
>>  	wrmsrl(MSR_CSTAR, ignore_sysret);
>> -	wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0);
>> +	wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0ULL);
> 
> Btw, we probably should define an invalid segment
> 
> #define GDT_ENTRY_INVALID_SEGMENT		0ULL
> 
> and use that value instead of the naked 0.

Why? Segment selectors aren't "long long", they are in fact "short"
(16-bit ints).

The "ULL" is used here in wrmsrl not because it's a segment,
but because wrmsrl expects a u64 parameter.

We can probably add a cast in its definition...



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value
  2015-04-03 11:52   ` Denys Vlasenko
@ 2015-04-03 12:05     ` Borislav Petkov
  2015-04-03 12:25       ` [PATCH] x86/asm/entry/64: Use a define for an invalid segment Borislav Petkov
  0 siblings, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2015-04-03 12:05 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Ingo Molnar, LKML, Andy Lutomirski, Alexei Starovoitov,
	Frederic Weisbecker, H. Peter Anvin, Kees Cook, Linus Torvalds,
	Oleg Nesterov, Steven Rostedt, Will Drewry

On Fri, Apr 03, 2015 at 01:52:00PM +0200, Denys Vlasenko wrote:
> Why? Segment selectors aren't "long long", they are in fact "short"
> (16-bit ints).
> 
> The "ULL" is used here in wrmsrl not because it's a segment,
> but because wrmsrl expects a u64 parameter.
> 
> We can probably add a cast in its definition...

Yeah, I meant to not use the naked 0. Cast should be fine.

Let me do a patch.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] x86/asm/entry/64: Use a define for an invalid segment
  2015-04-03 12:05     ` Borislav Petkov
@ 2015-04-03 12:25       ` Borislav Petkov
  0 siblings, 0 replies; 7+ messages in thread
From: Borislav Petkov @ 2015-04-03 12:25 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Ingo Molnar, LKML, Andy Lutomirski, Alexei Starovoitov,
	Frederic Weisbecker, H. Peter Anvin, Kees Cook, Linus Torvalds,
	Oleg Nesterov, Steven Rostedt, Will Drewry

On Fri, Apr 03, 2015 at 02:05:17PM +0200, Borislav Petkov wrote:
> Let me do a patch.

---
From: Borislav Petkov <bp@suse.de>
Date: Fri, 3 Apr 2015 14:22:00 +0200
Subject: [PATCH] x86/asm/entry/64: Use a define for an invalid segment
 selector

... instead of a naked number, for better readability.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/segment.h | 2 ++
 arch/x86/kernel/cpu/common.c   | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index d394899e055c..5a9856eb12ba 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -39,6 +39,8 @@
 /* ... GDT has it cleared */
 #define SEGMENT_GDT		0x0
 
+#define GDT_ENTRY_INVALID_SEG	0
+
 #ifdef CONFIG_X86_32
 /*
  * The layout of the per-CPU GDT under Linux:
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 2c249b57e4b5..a62cf04dac8a 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1223,7 +1223,7 @@ void syscall_init(void)
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)ia32_sysenter_target);
 #else
 	wrmsrl(MSR_CSTAR, ignore_sysret);
-	wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0ULL);
+	wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)GDT_ENTRY_INVALID_SEG);
 	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, 0ULL);
 #endif
-- 
2.3.3

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [tip:x86/asm] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value
  2015-04-03  9:42 [PATCH] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value Borislav Petkov
  2015-04-03 11:20 ` Borislav Petkov
@ 2015-04-03 14:07 ` tip-bot for Borislav Petkov
  2015-04-03 14:07 ` [tip:x86/asm] x86/asm/entry/64: Use a define for an invalid segment selector tip-bot for Borislav Petkov
  2 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Borislav Petkov @ 2015-04-03 14:07 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: torvalds, bp, rostedt, linux-kernel, luto, fweisbec, oleg,
	dvlasenk, wad, ast, keescook, mingo, tglx, hpa

Commit-ID:  7c74d5b7b7b63fc279ed446ebd78de20d81f2824
Gitweb:     http://git.kernel.org/tip/7c74d5b7b7b63fc279ed446ebd78de20d81f2824
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Fri, 3 Apr 2015 11:42:10 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 3 Apr 2015 15:29:12 +0200

x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value

Commit:

  d56fe4bf5f3c ("x86/asm/entry/64: Always set up SYSENTER MSRs")

missed to add "ULL" to the 0 and wrmsrl_safe() complains:

  arch/x86/kernel/cpu/common.c: In function ‘syscall_init’:
  arch/x86/kernel/cpu/common.c:1226:2: warning: right shift count >= width of type wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0);

Fix it.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1428054130-25847-1-git-send-email-bp@alien8.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index a383d53..d56d307 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1184,7 +1184,7 @@ void syscall_init(void)
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)ia32_sysenter_target);
 #else
 	wrmsrl(MSR_CSTAR, ignore_sysret);
-	wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0);
+	wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0ULL);
 	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, 0ULL);
 #endif

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [tip:x86/asm] x86/asm/entry/64: Use a define for an invalid segment selector
  2015-04-03  9:42 [PATCH] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value Borislav Petkov
  2015-04-03 11:20 ` Borislav Petkov
  2015-04-03 14:07 ` [tip:x86/asm] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value tip-bot for Borislav Petkov
@ 2015-04-03 14:07 ` tip-bot for Borislav Petkov
  2 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Borislav Petkov @ 2015-04-03 14:07 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: oleg, tglx, bp, fweisbec, keescook, bp, mingo, hpa, torvalds,
	linux-kernel, luto, rostedt, ast, dvlasenk, wad

Commit-ID:  6b51311c976593fb7311322b1647a912cd456ec4
Gitweb:     http://git.kernel.org/tip/6b51311c976593fb7311322b1647a912cd456ec4
Author:     Borislav Petkov <bp@alien8.de>
AuthorDate: Fri, 3 Apr 2015 14:25:28 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 3 Apr 2015 15:29:13 +0200

x86/asm/entry/64: Use a define for an invalid segment selector

... instead of a naked number, for better readability.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1428054130-25847-1-git-send-email-bp@alien8.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/segment.h | 2 ++
 arch/x86/kernel/cpu/common.c   | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index d394899..5a9856e 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -39,6 +39,8 @@
 /* ... GDT has it cleared */
 #define SEGMENT_GDT		0x0
 
+#define GDT_ENTRY_INVALID_SEG	0
+
 #ifdef CONFIG_X86_32
 /*
  * The layout of the per-CPU GDT under Linux:
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index d56d307..3f70538 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1184,7 +1184,7 @@ void syscall_init(void)
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)ia32_sysenter_target);
 #else
 	wrmsrl(MSR_CSTAR, ignore_sysret);
-	wrmsrl_safe(MSR_IA32_SYSENTER_CS, 0ULL);
+	wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)GDT_ENTRY_INVALID_SEG);
 	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, 0ULL);
 #endif

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-04-03 14:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-03  9:42 [PATCH] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value Borislav Petkov
2015-04-03 11:20 ` Borislav Petkov
2015-04-03 11:52   ` Denys Vlasenko
2015-04-03 12:05     ` Borislav Petkov
2015-04-03 12:25       ` [PATCH] x86/asm/entry/64: Use a define for an invalid segment Borislav Petkov
2015-04-03 14:07 ` [tip:x86/asm] x86/asm/entry/64: Fix MSR_IA32_SYSENTER_CS MSR value tip-bot for Borislav Petkov
2015-04-03 14:07 ` [tip:x86/asm] x86/asm/entry/64: Use a define for an invalid segment selector tip-bot for Borislav Petkov

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).