linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] RAS: two fixlets
@ 2017-10-02  9:28 Borislav Petkov
  2017-10-02  9:28 ` [PATCH 1/2] RAS/CEC: Use the right length for "cec_disable" Borislav Petkov
  2017-10-02  9:28 ` [PATCH 2/2] x86/mce: Hide mca_cfg Borislav Petkov
  0 siblings, 2 replies; 7+ messages in thread
From: Borislav Petkov @ 2017-10-02  9:28 UTC (permalink / raw)
  To: X86 ML; +Cc: LKML

From: Borislav Petkov <bp@suse.de>

Hi guys,

just minor stuff right now. Please queue.

Thx.

Borislav Petkov (1):
  x86/mce: Hide mca_cfg

Nicolas Iooss (1):
  RAS/CEC: Use the right length for "cec_disable"

 arch/x86/include/asm/mce.h                | 1 -
 arch/x86/kernel/cpu/mcheck/mce-internal.h | 7 +++++++
 arch/x86/kernel/cpu/mcheck/mce_amd.c      | 2 ++
 drivers/ras/cec.c                         | 2 +-
 4 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.13.0

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

* [PATCH 1/2] RAS/CEC: Use the right length for "cec_disable"
  2017-10-02  9:28 [PATCH 0/2] RAS: two fixlets Borislav Petkov
@ 2017-10-02  9:28 ` Borislav Petkov
  2017-10-02 15:42   ` Thomas Gleixner
  2017-10-02  9:28 ` [PATCH 2/2] x86/mce: Hide mca_cfg Borislav Petkov
  1 sibling, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2017-10-02  9:28 UTC (permalink / raw)
  To: X86 ML; +Cc: LKML

From: Nicolas Iooss <nicolas.iooss_linux@m4x.org>

parse_cec_param() compares a string with "cec_disable" using only 7
characters of the 11-character-long string. Fix the length.

Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
Fixes: 011d82611172 ("RAS: Add a Corrected Errors Collector")
Link: http://lkml.kernel.org/r/20170903075440.30250-1-nicolas.iooss_linux@m4x.org
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/ras/cec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c
index d0e5d6ee882c..586c296d1538 100644
--- a/drivers/ras/cec.c
+++ b/drivers/ras/cec.c
@@ -523,7 +523,7 @@ int __init parse_cec_param(char *str)
 	if (*str == '=')
 		str++;
 
-	if (!strncmp(str, "cec_disable", 7))
+	if (!strncmp(str, "cec_disable", 11))
 		ce_arr.disabled = 1;
 	else
 		return 0;
-- 
2.13.0

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

* [PATCH 2/2] x86/mce: Hide mca_cfg
  2017-10-02  9:28 [PATCH 0/2] RAS: two fixlets Borislav Petkov
  2017-10-02  9:28 ` [PATCH 1/2] RAS/CEC: Use the right length for "cec_disable" Borislav Petkov
@ 2017-10-02  9:28 ` Borislav Petkov
  2017-10-05 12:28   ` [tip:ras/urgent] " tip-bot for Borislav Petkov
  1 sibling, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2017-10-02  9:28 UTC (permalink / raw)
  To: X86 ML; +Cc: LKML

From: Borislav Petkov <bp@suse.de>

Now that lguest is gone, put it in the internal header which should be
used only by MCA/RAS code.

Add missing header guards while at it.

No functionality change.

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

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 181264989db5..8edac1de2e35 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -187,7 +187,6 @@ struct mca_msr_regs {
 
 extern struct mce_vendor_flags mce_flags;
 
-extern struct mca_config mca_cfg;
 extern struct mca_msr_regs msr_ops;
 
 enum mce_notifier_prios {
diff --git a/arch/x86/kernel/cpu/mcheck/mce-internal.h b/arch/x86/kernel/cpu/mcheck/mce-internal.h
index 098530a93bb7..debb974fd17d 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-internal.h
+++ b/arch/x86/kernel/cpu/mcheck/mce-internal.h
@@ -1,3 +1,6 @@
+#ifndef __X86_MCE_INTERNAL_H__
+#define __X86_MCE_INTERNAL_H__
+
 #include <linux/device.h>
 #include <asm/mce.h>
 
@@ -108,3 +111,7 @@ static inline void mce_work_trigger(void)	{ }
 static inline void mce_register_injector_chain(struct notifier_block *nb)	{ }
 static inline void mce_unregister_injector_chain(struct notifier_block *nb)	{ }
 #endif
+
+extern struct mca_config mca_cfg;
+
+#endif /* __X86_MCE_INTERNAL_H__ */
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 40e28ed77fbf..486f640b02ef 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -28,6 +28,8 @@
 #include <asm/msr.h>
 #include <asm/trace/irq_vectors.h>
 
+#include "mce-internal.h"
+
 #define NR_BLOCKS         5
 #define THRESHOLD_MAX     0xFFF
 #define INT_TYPE_APIC     0x00020000
-- 
2.13.0

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

* Re: [PATCH 1/2] RAS/CEC: Use the right length for "cec_disable"
  2017-10-02  9:28 ` [PATCH 1/2] RAS/CEC: Use the right length for "cec_disable" Borislav Petkov
@ 2017-10-02 15:42   ` Thomas Gleixner
  2017-10-03  9:04     ` Borislav Petkov
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Gleixner @ 2017-10-02 15:42 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: X86 ML, LKML

On Mon, 2 Oct 2017, Borislav Petkov wrote:
> From: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> 
> parse_cec_param() compares a string with "cec_disable" using only 7
> characters of the 11-character-long string. Fix the length.
> 
> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> Fixes: 011d82611172 ("RAS: Add a Corrected Errors Collector")
> Link: http://lkml.kernel.org/r/20170903075440.30250-1-nicolas.iooss_linux@m4x.org
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  drivers/ras/cec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c
> index d0e5d6ee882c..586c296d1538 100644
> --- a/drivers/ras/cec.c
> +++ b/drivers/ras/cec.c
> @@ -523,7 +523,7 @@ int __init parse_cec_param(char *str)
>  	if (*str == '=')
>  		str++;
>  
> -	if (!strncmp(str, "cec_disable", 7))
> +	if (!strncmp(str, "cec_disable", 11))

This kind of issue happens over and over. So if you really want to use
strncmp() then this should be:

#define	CEC_DISABLE	"cec_disable"

	if (!strncmp(str, CEC_DISABLE, strlen(CEC_DISABLE))

or we get a proper helper for that. Though in case of comparing some string
against a constant string strncmp() has no real advantage over strcmp() as
the comparison is guaranteed to be bound by the string constant.

Thanks,

	tglx

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

* Re: [PATCH 1/2] RAS/CEC: Use the right length for "cec_disable"
  2017-10-02 15:42   ` Thomas Gleixner
@ 2017-10-03  9:04     ` Borislav Petkov
  2017-10-07 14:29       ` Nicolas Iooss
  0 siblings, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2017-10-03  9:04 UTC (permalink / raw)
  To: Thomas Gleixner, Nicolas Iooss; +Cc: X86 ML, LKML

On Mon, Oct 02, 2017 at 05:42:56PM +0200, Thomas Gleixner wrote:
> On Mon, 2 Oct 2017, Borislav Petkov wrote:
> > From: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> > 
> > parse_cec_param() compares a string with "cec_disable" using only 7
> > characters of the 11-character-long string. Fix the length.
> > 
> > Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> > Fixes: 011d82611172 ("RAS: Add a Corrected Errors Collector")
> > Link: http://lkml.kernel.org/r/20170903075440.30250-1-nicolas.iooss_linux@m4x.org
> > Signed-off-by: Borislav Petkov <bp@suse.de>
> > ---
> >  drivers/ras/cec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c
> > index d0e5d6ee882c..586c296d1538 100644
> > --- a/drivers/ras/cec.c
> > +++ b/drivers/ras/cec.c
> > @@ -523,7 +523,7 @@ int __init parse_cec_param(char *str)
> >  	if (*str == '=')
> >  		str++;
> >  
> > -	if (!strncmp(str, "cec_disable", 7))
> > +	if (!strncmp(str, "cec_disable", 11))
> 
> This kind of issue happens over and over. So if you really want to use
> strncmp() then this should be:
> 
> #define	CEC_DISABLE	"cec_disable"
> 
> 	if (!strncmp(str, CEC_DISABLE, strlen(CEC_DISABLE))
> 
> or we get a proper helper for that. Though in case of comparing some string
> against a constant string strncmp() has no real advantage over strcmp() as
> the comparison is guaranteed to be bound by the string constant.

Right.

Nicolas, wanna address that?

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* [tip:ras/urgent] x86/mce: Hide mca_cfg
  2017-10-02  9:28 ` [PATCH 2/2] x86/mce: Hide mca_cfg Borislav Petkov
@ 2017-10-05 12:28   ` tip-bot for Borislav Petkov
  0 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Borislav Petkov @ 2017-10-05 12:28 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: bp, linux-kernel, tglx, hpa, mingo

Commit-ID:  262e681183ddcdb24d64a2f993e41a226adcec29
Gitweb:     https://git.kernel.org/tip/262e681183ddcdb24d64a2f993e41a226adcec29
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Mon, 2 Oct 2017 11:28:36 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 5 Oct 2017 14:23:06 +0200

x86/mce: Hide mca_cfg

Now that lguest is gone, put it in the internal header which should be
used only by MCA/RAS code.

Add missing header guards while at it.

No functional change.

Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20171002092836.22971-3-bp@alien8.de

---
 arch/x86/include/asm/mce.h                | 1 -
 arch/x86/kernel/cpu/mcheck/mce-internal.h | 7 +++++++
 arch/x86/kernel/cpu/mcheck/mce_amd.c      | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 1812649..8edac1d 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -187,7 +187,6 @@ struct mca_msr_regs {
 
 extern struct mce_vendor_flags mce_flags;
 
-extern struct mca_config mca_cfg;
 extern struct mca_msr_regs msr_ops;
 
 enum mce_notifier_prios {
diff --git a/arch/x86/kernel/cpu/mcheck/mce-internal.h b/arch/x86/kernel/cpu/mcheck/mce-internal.h
index 098530a..debb974 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-internal.h
+++ b/arch/x86/kernel/cpu/mcheck/mce-internal.h
@@ -1,3 +1,6 @@
+#ifndef __X86_MCE_INTERNAL_H__
+#define __X86_MCE_INTERNAL_H__
+
 #include <linux/device.h>
 #include <asm/mce.h>
 
@@ -108,3 +111,7 @@ static inline void mce_work_trigger(void)	{ }
 static inline void mce_register_injector_chain(struct notifier_block *nb)	{ }
 static inline void mce_unregister_injector_chain(struct notifier_block *nb)	{ }
 #endif
+
+extern struct mca_config mca_cfg;
+
+#endif /* __X86_MCE_INTERNAL_H__ */
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 40e28ed..486f640 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -28,6 +28,8 @@
 #include <asm/msr.h>
 #include <asm/trace/irq_vectors.h>
 
+#include "mce-internal.h"
+
 #define NR_BLOCKS         5
 #define THRESHOLD_MAX     0xFFF
 #define INT_TYPE_APIC     0x00020000

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

* Re: [PATCH 1/2] RAS/CEC: Use the right length for "cec_disable"
  2017-10-03  9:04     ` Borislav Petkov
@ 2017-10-07 14:29       ` Nicolas Iooss
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Iooss @ 2017-10-07 14:29 UTC (permalink / raw)
  To: Thomas Gleixner, Borislav Petkov; +Cc: X86 ML, LKML

On Tue, Oct 3, 2017 at 11:04 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Oct 02, 2017 at 05:42:56PM +0200, Thomas Gleixner wrote:
>> On Mon, 2 Oct 2017, Borislav Petkov wrote:
>> > From: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
>> >
>> > parse_cec_param() compares a string with "cec_disable" using only 7
>> > characters of the 11-character-long string. Fix the length.
>> >
>> > Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
>> > Fixes: 011d82611172 ("RAS: Add a Corrected Errors Collector")
>> > Link: http://lkml.kernel.org/r/20170903075440.30250-1-nicolas.iooss_linux@m4x.org
>> > Signed-off-by: Borislav Petkov <bp@suse.de>
>> > ---
>> >  drivers/ras/cec.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c
>> > index d0e5d6ee882c..586c296d1538 100644
>> > --- a/drivers/ras/cec.c
>> > +++ b/drivers/ras/cec.c
>> > @@ -523,7 +523,7 @@ int __init parse_cec_param(char *str)
>> >     if (*str == '=')
>> >             str++;
>> >
>> > -   if (!strncmp(str, "cec_disable", 7))
>> > +   if (!strncmp(str, "cec_disable", 11))
>>
>> This kind of issue happens over and over. So if you really want to use
>> strncmp() then this should be:
>>
>> #define       CEC_DISABLE     "cec_disable"
>>
>>       if (!strncmp(str, CEC_DISABLE, strlen(CEC_DISABLE))
>>
>> or we get a proper helper for that. Though in case of comparing some string
>> against a constant string strncmp() has no real advantage over strcmp() as
>> the comparison is guaranteed to be bound by the string constant.
>
> Right.
>
> Nicolas, wanna address that?
>
> Thx.

Hi, the only difference I see between strncmp+strlen and strcmp is
that the first option compares a prefix ("does the string begin with
cec_disable?") and the second one checks that strings are equals. In
parse_cec_param() it does not seem to matter.
I have seen that Thomas Gleixner has already committed a modified
patch which uses strcmp(), which looks good to me.

Thanks!
Nicolas

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

end of thread, other threads:[~2017-10-07 14:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-02  9:28 [PATCH 0/2] RAS: two fixlets Borislav Petkov
2017-10-02  9:28 ` [PATCH 1/2] RAS/CEC: Use the right length for "cec_disable" Borislav Petkov
2017-10-02 15:42   ` Thomas Gleixner
2017-10-03  9:04     ` Borislav Petkov
2017-10-07 14:29       ` Nicolas Iooss
2017-10-02  9:28 ` [PATCH 2/2] x86/mce: Hide mca_cfg Borislav Petkov
2017-10-05 12:28   ` [tip:ras/urgent] " 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).