Messages in this thread | | | Date | Wed, 22 Apr 2015 11:21:34 -0400 | From | Don Zickus <> | Subject | Re: [PATCH v9 2/3] watchdog: add watchdog_cpumask sysctl to assist nohz |
| |
On Wed, Apr 22, 2015 at 07:02:31AM -0400, Ulrich Obergfell wrote: > > 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.
Or we could just statically allocate it
static DECLARE_BITMAP(watchdog_cpumask, NR_CPUS) __read_mostly;
Cheers, Don
| |