All of lore.kernel.org
 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 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.