Re: [PATCH v9 2/3] watchdog: add watchdog_cpumask sysctl to assist nohz

[Ulrich Obergfell]

Chris,

in https://lkml.org/lkml/2015/4/17/616 you stated:

 ">> +	alloc_cpumask_var(&watchdog_cpumask_for_smpboot, GFP_KERNEL);
  >
  > alloc_cpumask_var could fail?

  Good catch; if I get a failure I'll just return early without trying to
  start the watchdog, since clearly things are too memory-constrained
  to enable that functionality anyway."

Let's assume that (in spite of the memory constraints) the kernel would still
be able to make progress and get to a point where the system will be usable.
In this corner case, the following code would leave a NULL pointer behind in
watchdog_cpumask and in watchdog_cpumask_bits which could subsequently lead
to a crash.

 void __init lockup_detector_init(void)
 {
         set_sample_period();
 
+        if (!alloc_cpumask_var(&watchdog_cpumask, GFP_KERNEL)) {
+                pr_err("Failed to allocate cpumask for watchdog");
+                return;
+        }
+        watchdog_cpumask_bits = cpumask_bits(watchdog_cpumask);

For example, proc_watchdog_cpumask() and the change that your patch introduces
in watchdog_enable_all_cpus() are not protected against a possible NULL pointer.
I think the code needs to be made safer.


Regards,

Uli
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/