lkml.org 
[lkml]   [2014]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] EDAC, MCE, AMD: Fix code to prevent NULL dereference
On 2/18/2014 2:46 AM, Borislav Petkov wrote:
> Ok, let's try a simpler thing. Only build-tested here:
>
>
> + if (!fam_ops)
> + return NOTIFY_DONE;
> +
> if (amd_filter_mce(m))
> return NOTIFY_STOP;
>
> @@ -816,10 +819,10 @@ static int __init mce_amd_init(void)
> struct cpuinfo_x86 *c = &boot_cpu_data;
>
> if (c->x86_vendor != X86_VENDOR_AMD)
> - return 0;
> + return -ENODEV;
>
> if (c->x86 < 0xf || c->x86 > 0x16)
> - return 0;
> + return -ENODEV;
>
> fam_ops = kzalloc(sizeof(struct amd_decoder_ops), GFP_KERNEL);
> if (!fam_ops)
> @@ -874,6 +877,7 @@ static int __init mce_amd_init(void)
> default:
> printk(KERN_WARNING "Huh? What family is it: 0x%x?!\n", c->x86);
> kfree(fam_ops);
> + fam_ops = NULL;
> return -EINVAL;
> }
>
>

This works. But a drawback is that you wouldn't get the output from more
generic error decoding that happens after the 'switch' in amd_decode_mce:

pr_emerg(HW_ERR "Error Status: %s\n", decode_error_status(m))
(etc..) (etc..)
amd_decode_err_code(m->status & 0xffff);

A quick fix for this is to rearrange the above chunk of code to happen
before the 'switch'
Tried it on local machine.Here's some sample outputs:

on unsupported h/w:
[ 46.822828] [Hardware Error]: Error Status: Uncorrected, software
containable error.
[ 46.822846] [Hardware Error]: CPU:0 (15:30:0)
MC0_STATUS[-|UE|-|-|-|-|-]: 0xa000000000010f0f
[ 46.822858] [Hardware Error]: cache level: L3/GEN, mem/io: GEN,
mem-tx: GEN, part-proc: GEN (timed out)

on supported h/w:(a MC0 error)
[ 84.305292] [Hardware Error]: Error Status: Uncorrected, software
containable error.
[ 84.305312] [Hardware Error]: CPU:0 (15:30:0)
MC0_STATUS[-|UE|-|-|-|-|-]: 0xa000000000010f0f
[ 84.305327] [Hardware Error]: cache level: L3/GEN, mem/io: GEN,
mem-tx: GEN, part-proc: GEN (timed out)
[ 84.305343] [Hardware Error]: MC0 Error: Internal error condition
type 1.

on supported h/w:(a MC4 ECC error)
[ 128.942878] [Hardware Error]: Error Status: System Fatal error.
[ 128.942897] [Hardware Error]: CPU:0 (15:30:0)
MC4_STATUS[-|UE|-|PCC|AddrV|-|-|UECC]: 0xa600200000080a23
[ 128.942914] [Hardware Error]: MC4_ADDR: 0x0000000000000000
[ 128.942922] [Hardware Error]: cache level: L3/GEN, mem/io: MEM,
mem-tx: WR, part-proc: RES (no timeout)
[ 128.942939] [Hardware Error]: MC4 Error (node 0): DRAM ECC error
detected on the NB.
[ 128.942971] EDAC MC0: 1 UE on mc#0csrow#2channel#0 (csrow:2 channel:0
page:0x0 offset:0x0 grain:0)

A word about your earlier suggestion of using amd_notifier_call_chain in
mce_amd_inj:
The changes will need to be more involved..
- Firstly, x86_mce_decoder_chain is defined in mce.c. So we'd need to
move it to somewhere in asm/mce.h
- include notifier.h in asm/mce.h (build error saying there are multiple
definitions of 'x86_mce_decoder_chain' when I tried this.. haven't
figured out why yet..)
- You'd need to change i_mce to pointer type which in turn will need
changes in the manner we reference the struct variables in the code

Not sure if you need these many changes, not to mention - touch common
mce code.
Simpler solution might be to rearrange the code in amd_decode_mce and
use your hunk..
Thoughts?

Thanks,
-Aravind.




\
 
 \ /
  Last update: 2014-02-18 20:01    [W:0.122 / U:2.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site