[[linux-yocto] [PATCH] kernel/sched: Fix double free on invalid isolcpus/nohz_full params] On 17/08/2023 (Thu 10:56) Adrian Cinal via lists.yoctoproject.org wrote:
> A previous patch left behind a redundant call to free_bootmem_cpumask_var > possibly leading to a double free (once in the if-branch and once in the > unwind code at the end of the function) if the isolcpus= or nohz_full= > kernel command line parameters failed validation, cf.: > > https://lists.yoctoproject.org/g/linux-yocto/message/12797 Once again, I wanted to know exactly how we got here. So here is how. The key to this issue is this commit from v5.18: commit 0cd3e59de1f53978873669c7c8225ec13e88c3ae Author: Frederic Weisbecker <frede...@kernel.org> Date: Mon Feb 7 16:59:08 2022 +0100 sched/isolation: Consolidate error handling Centralize the mask freeing and return value for the error path. This makes potential leaks more visible. Simple and common enough; it contains multiple instances of: - free_bootmem_cpumask_var(non_housekeeping_mask); - return 0; + goto free_non_housekeeping_mask; But from a kernel version persepective, it means we have an "old style" free and the "new style" free. The patch sent to lkml was for post 5.18 kernels, and used the new style, and was even documented as such in the 0/N (see asteriks): ------------- This is a rebase and retest of two fixes I'd sent earlier[1]. The rebase is required due to conflicts in my patch #1 and where Frederic updated the unwind code in housekeeping_setup in his series[2] and that series is now in sched/core of tip[3]. So this update is against a baseline of ed3b362d54f0 found in sched/core as "sched/isolation: Split housekeeping cpumask per isolation features" in tip. ************** Changes amount to "return 0" ---> "goto out_free" and adding a nod to PaulM's observation that nohz_full w/o a cpuset is coming someday into the commit log. ************** [1] https://lore.kernel.org/all/20211206145950.10927-1-paul.gortma...@windriver.com/ [2] https://lore.kernel.org/all/20220207155910.527133-1-frede...@kernel.org/ [3] git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git ------------- https://lore.kernel.org/lkml/20220221182009.1283-1-paul.gortma...@windriver.com/ Similarly, the submission for yocto for the v5.15 kernel (which was current at the time) used the old style and everything was fine. When yocto moved to v6.1, the uprev generally carries forward commits that have not been merged to mainline or declared obsolete. So the v5.15 old style commit was carried forward to v6.1, resulting in a mix of old and new style free. Not ideal, but still functionally correct. The double free risk comes from your change which deleted the "return 0" underneath the old free: https://lists.yoctoproject.org/g/linux-yocto/topic/99772129 It shouldn't have done that, and I missed it in review. Bruce: the executive summary is that this delta fix is correct, and should be placed on the 6.1/6.4 branches that got the previous commit from Adrian. The yocto-kernel-cache can and should ignore all this churn, and simply contain the post v5.18 "new style" version of the commit sent to lkml: https://lore.kernel.org/lkml/20220221182009.1283-2-paul.gortma...@windriver.com/ Thanks, Paul. > > Signed-off-by: Adrian Cinal <adriancin...@gmail.com> > --- > kernel/sched/isolation.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c > index b97d6e05013d..7bebfdc42486 100644 > --- a/kernel/sched/isolation.c > +++ b/kernel/sched/isolation.c > @@ -133,7 +133,6 @@ static int __init housekeeping_setup(char *str, unsigned > long flags) > > if (cpumask_empty(non_housekeeping_mask)) { > pr_info("housekeeping: kernel parameter 'nohz_full=' or > 'isolcpus=' has no valid CPUs.\n"); > - free_bootmem_cpumask_var(non_housekeeping_mask); > goto free_non_housekeeping_mask; > } > > -- > 2.41.0 > > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#12998): https://lists.yoctoproject.org/g/linux-yocto/message/12998 Mute This Topic: https://lists.yoctoproject.org/mt/100796885/21656 Group Owner: linux-yocto+ow...@lists.yoctoproject.org Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
