[PATCH] x86/mce: Check misc_register() return code.
The current code does not check the return from misc_register() so set the return variable with the return code of misc_register. Signed-off-by: Mathieu Souchaud --- arch/x86/kernel/cpu/mcheck/mce.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index 68317c8..45fb7aa 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -2462,7 +2462,7 @@ static __init int mcheck_init_device(void) cpu_notifier_register_done(); /* register character device /dev/mcelog */ - misc_register(&mce_chrdev_device); + err = misc_register(&mce_chrdev_device); return err; } -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/mce: Check misc_register() return code.
Great, I will fix that soon. Thanks. Le 25/04/2014 12:42, Borislav Petkov a écrit : On Fri, Apr 25, 2014 at 11:48:15AM +0200, Mathieu Souchaud wrote: The current code does not check the return from misc_register() so set the return variable with the return code of misc_register. Signed-off-by: Mathieu Souchaud --- arch/x86/kernel/cpu/mcheck/mce.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index 68317c8..45fb7aa 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -2462,7 +2462,7 @@ static __init int mcheck_init_device(void) cpu_notifier_register_done(); /* register character device /dev/mcelog */ - misc_register(&mce_chrdev_device); + err = misc_register(&mce_chrdev_device); This function mcheck_init_device() is a one big sloppy mess of not handling the error cases responsibly. It doesn't check zalloc_cpumask_var() retval, it doesn't free the mce_device_initialized when subsys_system_register() fails, the same when mce_device_create fails, the same with __register_hotcpu_notifier and finally with misc_register. Would you be willing to fix this properly with goto labels and proper unwinding, like it is done in other, saner places in the kernel, and at the end maybe add a printk for the error case warning us that /dev/mcelog didn't get registered? That would be a very nice cleanup! :-) Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86/mce: Improve mcheck_init_device() error handling.
Check return code of every function called.
Signed-off-by: Mathieu Souchaud
---
arch/x86/kernel/cpu/mcheck/mce.c | 30 +++---
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 68317c8..b865285 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2437,33 +2437,49 @@ static __init int mcheck_init_device(void)
int err;
int i = 0;
- if (!mce_available(&boot_cpu_data))
- return -EIO;
+ if (!mce_available(&boot_cpu_data)) {
+ err = -EIO;
+ goto err_out;
+ }
- zalloc_cpumask_var(&mce_device_initialized, GFP_KERNEL);
+ if (!zalloc_cpumask_var(&mce_device_initialized, GFP_KERNEL)) {
+ err = -ENOMEM;
+ goto err_out;
+ }
mce_init_banks();
err = subsys_system_register(&mce_subsys, NULL);
if (err)
- return err;
+ goto err_out_mem;
cpu_notifier_register_begin();
for_each_online_cpu(i) {
err = mce_device_create(i);
if (err) {
cpu_notifier_register_done();
- return err;
+ goto err_out_mem;
}
}
register_syscore_ops(&mce_syscore_ops);
- __register_hotcpu_notifier(&mce_cpu_notifier);
+ err = __register_hotcpu_notifier(&mce_cpu_notifier);
cpu_notifier_register_done();
+ if (err)
+ goto err_out_mem;
/* register character device /dev/mcelog */
- misc_register(&mce_chrdev_device);
+ err = misc_register(&mce_chrdev_device);
+ if (err)
+ goto err_out_mem;
+
+ return 0;
+
+err_out_mem:
+ free_cpumask_var(mce_device_initialized);
+err_out:
+ pr_err("Unable to init device /dev/mcelog (rc: %d)\n", err);
return err;
}
device_initcall_sync(mcheck_init_device);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/mce: Improve mcheck_init_device() error handling.
In this patch, unwindind is still not perfect from the
mce_device_create() calls. I guess, from this point, on error we should
call mce_device_remove on every created device. But I think this will
make the code rather complex. And I am not too confident on this kind of
modification as I don't know much about MCE.
Mathieu S.
Le 25/04/2014 19:40, Mathieu Souchaud a écrit :
Check return code of every function called.
Signed-off-by: Mathieu Souchaud
---
arch/x86/kernel/cpu/mcheck/mce.c | 30 +++---
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 68317c8..b865285 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2437,33 +2437,49 @@ static __init int mcheck_init_device(void)
int err;
int i = 0;
- if (!mce_available(&boot_cpu_data))
- return -EIO;
+ if (!mce_available(&boot_cpu_data)) {
+ err = -EIO;
+ goto err_out;
+ }
- zalloc_cpumask_var(&mce_device_initialized, GFP_KERNEL);
+ if (!zalloc_cpumask_var(&mce_device_initialized, GFP_KERNEL)) {
+ err = -ENOMEM;
+ goto err_out;
+ }
mce_init_banks();
err = subsys_system_register(&mce_subsys, NULL);
if (err)
- return err;
+ goto err_out_mem;
cpu_notifier_register_begin();
for_each_online_cpu(i) {
err = mce_device_create(i);
if (err) {
cpu_notifier_register_done();
- return err;
+ goto err_out_mem;
}
}
register_syscore_ops(&mce_syscore_ops);
- __register_hotcpu_notifier(&mce_cpu_notifier);
+ err = __register_hotcpu_notifier(&mce_cpu_notifier);
cpu_notifier_register_done();
+ if (err)
+ goto err_out_mem;
/* register character device /dev/mcelog */
- misc_register(&mce_chrdev_device);
+ err = misc_register(&mce_chrdev_device);
+ if (err)
+ goto err_out_mem;
+
+ return 0;
+
+err_out_mem:
+ free_cpumask_var(mce_device_initialized);
+err_out:
+ pr_err("Unable to init device /dev/mcelog (rc: %d)\n", err);
return err;
}
device_initcall_sync(mcheck_init_device);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] x86/mce: Improve mcheck_init_device() error handling
Check return code of every function called by mcheck_init_device().
Signed-off-by: Mathieu Souchaud
---
arch/x86/kernel/cpu/mcheck/mce.c | 49 +++---
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 68317c8..284cfad 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2437,32 +2437,65 @@ static __init int mcheck_init_device(void)
int err;
int i = 0;
- if (!mce_available(&boot_cpu_data))
- return -EIO;
+ if (!mce_available(&boot_cpu_data)) {
+ err = -EIO;
+ goto err_out;
+ }
- zalloc_cpumask_var(&mce_device_initialized, GFP_KERNEL);
+ if (!zalloc_cpumask_var(&mce_device_initialized, GFP_KERNEL)) {
+ err = -ENOMEM;
+ goto err_out;
+ }
mce_init_banks();
err = subsys_system_register(&mce_subsys, NULL);
if (err)
- return err;
+ goto err_out_mem;
cpu_notifier_register_begin();
for_each_online_cpu(i) {
err = mce_device_create(i);
if (err) {
- cpu_notifier_register_done();
- return err;
+ goto err_device_create;
}
}
register_syscore_ops(&mce_syscore_ops);
- __register_hotcpu_notifier(&mce_cpu_notifier);
+ err = __register_hotcpu_notifier(&mce_cpu_notifier);
+ if (err)
+ goto err_reg_hotcpu;
cpu_notifier_register_done();
/* register character device /dev/mcelog */
- misc_register(&mce_chrdev_device);
+ err = misc_register(&mce_chrdev_device);
+ if (err)
+ goto err_register;
+
+ return 0;
+
+
+err_register:
+ cpu_notifier_register_begin();
+ __unregister_hotcpu_notifier(&mce_cpu_notifier);
+
+err_reg_hotcpu:
+ unregister_syscore_ops(&mce_syscore_ops);
+
+err_device_create:
+ /*
+* mce_device_remove behave properly if mce_device_create was not
+* called on that device.
+*/
+ for_each_possible_cpu(i)
+ mce_device_remove(i);
+ cpu_notifier_register_done();
+
+err_out_mem:
+ free_cpumask_var(mce_device_initialized);
+
+err_out:
+ pr_err("Unable to init device /dev/mcelog (rc: %d)\n", err);
return err;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86/mce: Improve mcheck_init_device() error handling
I took some time to make a light kvm guest and "inject" errors. It seems to
work fine (after solving a deadlock issue :) ).
Mathieu S.
Le 03/05/2014 23:03, Mathieu Souchaud a écrit :
Check return code of every function called by mcheck_init_device().
Signed-off-by: Mathieu Souchaud
---
arch/x86/kernel/cpu/mcheck/mce.c | 49 +++---
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 68317c8..284cfad 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2437,32 +2437,65 @@ static __init int mcheck_init_device(void)
int err;
int i = 0;
- if (!mce_available(&boot_cpu_data))
- return -EIO;
+ if (!mce_available(&boot_cpu_data)) {
+ err = -EIO;
+ goto err_out;
+ }
- zalloc_cpumask_var(&mce_device_initialized, GFP_KERNEL);
+ if (!zalloc_cpumask_var(&mce_device_initialized, GFP_KERNEL)) {
+ err = -ENOMEM;
+ goto err_out;
+ }
mce_init_banks();
err = subsys_system_register(&mce_subsys, NULL);
if (err)
- return err;
+ goto err_out_mem;
cpu_notifier_register_begin();
for_each_online_cpu(i) {
err = mce_device_create(i);
if (err) {
- cpu_notifier_register_done();
- return err;
+ goto err_device_create;
}
}
register_syscore_ops(&mce_syscore_ops);
- __register_hotcpu_notifier(&mce_cpu_notifier);
+ err = __register_hotcpu_notifier(&mce_cpu_notifier);
+ if (err)
+ goto err_reg_hotcpu;
cpu_notifier_register_done();
/* register character device /dev/mcelog */
- misc_register(&mce_chrdev_device);
+ err = misc_register(&mce_chrdev_device);
+ if (err)
+ goto err_register;
+
+ return 0;
+
+
+err_register:
+ cpu_notifier_register_begin();
+ __unregister_hotcpu_notifier(&mce_cpu_notifier);
+
+err_reg_hotcpu:
+ unregister_syscore_ops(&mce_syscore_ops);
+
+err_device_create:
+ /*
+* mce_device_remove behave properly if mce_device_create was not
+* called on that device.
+*/
+ for_each_possible_cpu(i)
+ mce_device_remove(i);
+ cpu_notifier_register_done();
+
+err_out_mem:
+ free_cpumask_var(mce_device_initialized);
+
+err_out:
+ pr_err("Unable to init device /dev/mcelog (rc: %d)\n", err);
return err;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3] x86/mce: Improve mcheck_init_device() error handling
Check return code of every function called by mcheck_init_device().
Signed-off-by: Mathieu Souchaud
---
arch/x86/kernel/cpu/mcheck/mce.c | 47 --
1 file changed, 40 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 68317c8..e361681 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2437,32 +2437,65 @@ static __init int mcheck_init_device(void)
int err;
int i = 0;
- if (!mce_available(&boot_cpu_data))
- return -EIO;
+ if (!mce_available(&boot_cpu_data)) {
+ err = -EIO;
+ goto err_out;
+ }
- zalloc_cpumask_var(&mce_device_initialized, GFP_KERNEL);
+ if (!zalloc_cpumask_var(&mce_device_initialized, GFP_KERNEL)) {
+ err = -ENOMEM;
+ goto err_out;
+ }
mce_init_banks();
err = subsys_system_register(&mce_subsys, NULL);
if (err)
- return err;
+ goto err_out_mem;
cpu_notifier_register_begin();
for_each_online_cpu(i) {
err = mce_device_create(i);
if (err) {
cpu_notifier_register_done();
- return err;
+ goto err_device_create;
}
}
- register_syscore_ops(&mce_syscore_ops);
__register_hotcpu_notifier(&mce_cpu_notifier);
cpu_notifier_register_done();
+ register_syscore_ops(&mce_syscore_ops);
+
/* register character device /dev/mcelog */
- misc_register(&mce_chrdev_device);
+ err = misc_register(&mce_chrdev_device);
+ if (err)
+ goto err_register;
+
+ return 0;
+
+err_register:
+ unregister_syscore_ops(&mce_syscore_ops);
+
+ cpu_notifier_register_begin();
+ __unregister_hotcpu_notifier(&mce_cpu_notifier);
+ cpu_notifier_register_done();
+
+err_device_create:
+ /*
+* We didn't keep track of which devices were created above, but
+* even if we had, the set of online cpus might have changed.
+* Play safe and remove for every possible cpu, since
+* mce_device_remove() will do the right thing.
+*/
+ for_each_possible_cpu(i)
+ mce_device_remove(i);
+
+err_out_mem:
+ free_cpumask_var(mce_device_initialized);
+
+err_out:
+ pr_err("Unable to init device /dev/mcelog (rc: %d)\n", err);
return err;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] x86/mce: Improve mcheck_init_device() error handling
Thanks too, you helped me a lot. I can do more work, if it's not too difficult for me. Mathieu Le 28/05/2014 11:52, Borislav Petkov a écrit : On Wed, May 28, 2014 at 09:12:37AM +0200, Mathieu Souchaud wrote: Check return code of every function called by mcheck_init_device(). Signed-off-by: Mathieu Souchaud Applied, thanks! I'll queue it for 3.17 though since we're very close to the 3.16 merge window and this patch hasn't had any linux-next time. Thanks for your good work, let me know if you want to do more around mce. :-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
