All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.