[3/6] x86/cpu: proc - remove "wp" status line in cpuinfo
diff mbox series

Message ID 1486933932-585-4-git-send-email-minipli@googlemail.com
State New, archived
Headers show
Series
  • struct cpuinfo_x86 related cleanups
Related show

Commit Message

Mathias Krause Feb. 12, 2017, 9:12 p.m. UTC
As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
correctly. This makes a process reading this file always see "wp : yes"
here -- otherwise there would be no process to begin with ;)

As this status line in /proc/cpuinfo serves no purpose for quite some
time now, get rid of it.

Cc: Borislav Petkov <bp@alien8.de>
Cc: H. Peter Anvin <hpa@linux.intel.com>
Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 arch/x86/kernel/cpu/proc.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Borislav Petkov Feb. 14, 2017, 4:20 p.m. UTC | #1
On Sun, Feb 12, 2017 at 10:12:09PM +0100, Mathias Krause wrote:
> As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
> CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
> correctly. This makes a process reading this file always see "wp : yes"
> here -- otherwise there would be no process to begin with ;)
> 
> As this status line in /proc/cpuinfo serves no purpose for quite some
> time now, get rid of it.

Right, sure, except /proc/cpuinfo's format is kind of an ABI and scripts
rely on it, I'm being told. TBH, I'd remove that wp:-line too but this
is just me. tip guys' call.

FWIW, for all three:

Acked-by: Borislav Petkov <bp@suse.de>
Mathias Krause Feb. 14, 2017, 4:47 p.m. UTC | #2
On 14 February 2017 at 17:20, Borislav Petkov <bp@alien8.de> wrote:
> On Sun, Feb 12, 2017 at 10:12:09PM +0100, Mathias Krause wrote:
>> As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
>> CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
>> correctly. This makes a process reading this file always see "wp : yes"
>> here -- otherwise there would be no process to begin with ;)
>>
>> As this status line in /proc/cpuinfo serves no purpose for quite some
>> time now, get rid of it.
>
> Right, sure, except /proc/cpuinfo's format is kind of an ABI and scripts
> rely on it, I'm being told.

That's the reason I haven't folded this change into patch 2. I had
similar doubts but it's not documented in Documentation/ and kinda
useless to test anyway -- what would a "wp : no" tell one?

> TBH, I'd remove that wp:-line too but this
> is just me. tip guys' call.
>
> FWIW, for all three:
>
> Acked-by: Borislav Petkov <bp@suse.de>

Thanks,
Mathias
Borislav Petkov Feb. 14, 2017, 5:11 p.m. UTC | #3
On Tue, Feb 14, 2017 at 05:47:08PM +0100, Mathias Krause wrote:
> That's the reason I haven't folded this change into patch 2. I had
> similar doubts but it's not documented in Documentation/ and kinda
> useless to test anyway -- what would a "wp : no" tell one?

Not that - the missing wp-line in there might puzzle some idiotic
userspace script. And then it is our fault all over again that we broke
the world.

But I'm just playing the devil's advocate here. Realistically, it is
very likely that no one would care.
H. Peter Anvin Feb. 14, 2017, 6:13 p.m. UTC | #4
On 02/12/17 13:12, Mathias Krause wrote:
> As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
> CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
> correctly. This makes a process reading this file always see "wp : yes"
> here -- otherwise there would be no process to begin with ;)
> 
> As this status line in /proc/cpuinfo serves no purpose for quite some
> time now, get rid of it.
> 
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: H. Peter Anvin <hpa@linux.intel.com>
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> ---
>  arch/x86/kernel/cpu/proc.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
> index 6df621ae62a7..c6c5217a7980 100644
> --- a/arch/x86/kernel/cpu/proc.c
> +++ b/arch/x86/kernel/cpu/proc.c
> @@ -30,8 +30,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>  		   "coma_bug\t: %s\n"
>  		   "fpu\t\t: %s\n"
>  		   "fpu_exception\t: %s\n"
> -		   "cpuid level\t: %d\n"
> -		   "wp\t\t: yes\n",
> +		   "cpuid level\t: %d\n",
>  		   static_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
>  		   static_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
>  		   static_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
> @@ -45,8 +44,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>  	seq_printf(m,
>  		   "fpu\t\t: yes\n"
>  		   "fpu_exception\t: yes\n"
> -		   "cpuid level\t: %d\n"
> -		   "wp\t\t: yes\n",
> +		   "cpuid level\t: %d\n",
>  		   c->cpuid_level);
>  }
>  #endif
> 

Potential userspace breakage, which is why the line was left in the
first place despite its value now being hard-coded.  Removing it will
save a whopping 9 bytes of kernel rodata; this is a very small price to
pay for guaranteeing continued compatibility.

Nacked-by: H. Peter Anvin <hpa@zytor.com>
Borislav Petkov Feb. 14, 2017, 6:30 p.m. UTC | #5
On Tue, Feb 14, 2017 at 10:13:22AM -0800, H. Peter Anvin wrote:
> Potential userspace breakage, which is why the line was left in the

We could keep the string in /proc/cpuinfo for compatibility but kill the
cpuinfo_x86 members.
Mathias Krause Feb. 14, 2017, 9:42 p.m. UTC | #6
On 14 February 2017 at 19:13, H. Peter Anvin <hpa@zytor.com> wrote:
> On 02/12/17 13:12, Mathias Krause wrote:
>> As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
>> CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
>> correctly. This makes a process reading this file always see "wp : yes"
>> here -- otherwise there would be no process to begin with ;)
>>
>> As this status line in /proc/cpuinfo serves no purpose for quite some
>> time now, get rid of it.
>>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: H. Peter Anvin <hpa@linux.intel.com>
>> Signed-off-by: Mathias Krause <minipli@googlemail.com>
>> ---
>>  arch/x86/kernel/cpu/proc.c |    6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
>> index 6df621ae62a7..c6c5217a7980 100644
>> --- a/arch/x86/kernel/cpu/proc.c
>> +++ b/arch/x86/kernel/cpu/proc.c
>> @@ -30,8 +30,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>>                  "coma_bug\t: %s\n"
>>                  "fpu\t\t: %s\n"
>>                  "fpu_exception\t: %s\n"
>> -                "cpuid level\t: %d\n"
>> -                "wp\t\t: yes\n",
>> +                "cpuid level\t: %d\n",
>>                  static_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
>>                  static_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
>>                  static_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
>> @@ -45,8 +44,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>>       seq_printf(m,
>>                  "fpu\t\t: yes\n"
>>                  "fpu_exception\t: yes\n"
>> -                "cpuid level\t: %d\n"
>> -                "wp\t\t: yes\n",
>> +                "cpuid level\t: %d\n",
>>                  c->cpuid_level);
>>  }
>>  #endif
>>
>
> Potential userspace breakage, which is why the line was left in the
> first place despite its value now being hard-coded.  Removing it will
> save a whopping 9 bytes of kernel rodata; this is a very small price to
> pay for guaranteeing continued compatibility.

Indeed. That's why I've separated the removal into an extra patch --
to make it easier not to take it.

>
> Nacked-by: H. Peter Anvin <hpa@zytor.com>

Do you want me to send the series again without this patch and patch
#6 (Geert took it already) or are you okay with sorting them out
yourself?

Cheers,
Mathias
Mathias Krause Feb. 28, 2017, 7:15 a.m. UTC | #7
On 14 February 2017 at 22:42, Mathias Krause <minipli@googlemail.com> wrote:
> On 14 February 2017 at 19:13, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 02/12/17 13:12, Mathias Krause wrote:
>>> As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
>>> CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
>>> correctly. This makes a process reading this file always see "wp : yes"
>>> here -- otherwise there would be no process to begin with ;)
>>>
>>> As this status line in /proc/cpuinfo serves no purpose for quite some
>>> time now, get rid of it.
>>>
>>> Cc: Borislav Petkov <bp@alien8.de>
>>> Cc: H. Peter Anvin <hpa@linux.intel.com>
>>> Signed-off-by: Mathias Krause <minipli@googlemail.com>
>>> ---
>>>  arch/x86/kernel/cpu/proc.c |    6 ++----
>>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
>>> index 6df621ae62a7..c6c5217a7980 100644
>>> --- a/arch/x86/kernel/cpu/proc.c
>>> +++ b/arch/x86/kernel/cpu/proc.c
>>> @@ -30,8 +30,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>>>                  "coma_bug\t: %s\n"
>>>                  "fpu\t\t: %s\n"
>>>                  "fpu_exception\t: %s\n"
>>> -                "cpuid level\t: %d\n"
>>> -                "wp\t\t: yes\n",
>>> +                "cpuid level\t: %d\n",
>>>                  static_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
>>>                  static_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
>>>                  static_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
>>> @@ -45,8 +44,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>>>       seq_printf(m,
>>>                  "fpu\t\t: yes\n"
>>>                  "fpu_exception\t: yes\n"
>>> -                "cpuid level\t: %d\n"
>>> -                "wp\t\t: yes\n",
>>> +                "cpuid level\t: %d\n",
>>>                  c->cpuid_level);
>>>  }
>>>  #endif
>>>
>>
>> Potential userspace breakage, which is why the line was left in the
>> first place despite its value now being hard-coded.  Removing it will
>> save a whopping 9 bytes of kernel rodata; this is a very small price to
>> pay for guaranteeing continued compatibility.
>
> Indeed. That's why I've separated the removal into an extra patch --
> to make it easier not to take it.
>
>>
>> Nacked-by: H. Peter Anvin <hpa@zytor.com>
>
> Do you want me to send the series again without this patch and patch
> #6 (Geert took it already) or are you okay with sorting them out
> yourself?
>

Ping...
Peter, what's your preference here?

Mathias

Patch
diff mbox series

diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
index 6df621ae62a7..c6c5217a7980 100644
--- a/arch/x86/kernel/cpu/proc.c
+++ b/arch/x86/kernel/cpu/proc.c
@@ -30,8 +30,7 @@  static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
 		   "coma_bug\t: %s\n"
 		   "fpu\t\t: %s\n"
 		   "fpu_exception\t: %s\n"
-		   "cpuid level\t: %d\n"
-		   "wp\t\t: yes\n",
+		   "cpuid level\t: %d\n",
 		   static_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
 		   static_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
 		   static_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
@@ -45,8 +44,7 @@  static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
 	seq_printf(m,
 		   "fpu\t\t: yes\n"
 		   "fpu_exception\t: yes\n"
-		   "cpuid level\t: %d\n"
-		   "wp\t\t: yes\n",
+		   "cpuid level\t: %d\n",
 		   c->cpuid_level);
 }
 #endif