-----Original Message----- From: Haiyang Zhang haiyangz@microsoft.com Sent: Sunday, January 29, 2023 1:51 PM To: Michael Kelley (LINUX) mikelley@microsoft.com; linux- hyperv@vger.kernel.org; netdev@vger.kernel.org Cc: Dexuan Cui decui@microsoft.com; KY Srinivasan kys@microsoft.com; Paul Rosswurm paulros@microsoft.com; olaf@aepfle.de; vkuznets@redhat.com; davem@davemloft.net; linux-kernel@vger.kernel.org; stable@vger.kernel.org Subject: RE: [PATCH net, 2/2] net: mana: Fix accessing freed irq affinity_hint
-----Original Message----- From: Michael Kelley (LINUX) mikelley@microsoft.com Sent: Sunday, January 29, 2023 9:27 AM To: Haiyang Zhang haiyangz@microsoft.com; linux-
hyperv@vger.kernel.org;
netdev@vger.kernel.org Cc: Haiyang Zhang haiyangz@microsoft.com; Dexuan Cui decui@microsoft.com; KY Srinivasan kys@microsoft.com; Paul
Rosswurm
paulros@microsoft.com; olaf@aepfle.de; vkuznets@redhat.com; davem@davemloft.net; linux-kernel@vger.kernel.org;
stable@vger.kernel.org
Subject: RE: [PATCH net, 2/2] net: mana: Fix accessing freed irq affinity_hint
From: LKML haiyangz lkmlhyz@microsoft.com On Behalf Of Haiyang
Zhang
Sent: Thursday, January 26, 2023 1:05 PM
After calling irq_set_affinity_and_hint(), the cpumask pointer is saved in desc->affinity_hint, and will be used later when reading /proc/irq/<num>/affinity_hint. So the cpumask variable needs to be allocated per irq, and available until freeing the irq. Otherwise, we are accessing freed memory when reading the affinity_hint file.
To fix the bug, allocate the cpumask per irq, and free it just before freeing the irq.
Since the cpumask being passed to irq_set_affinity_and_hint() always contains exactly one CPU, the code can be considerably simplified by using the pre-calculated and persistent masks available as cpumask_of(cpu). All allocation of cpumasks in this code goes away, and you can set the affinity_hint to NULL in the cleanup and remove paths without having to free any masks.
Great idea! Will update the patch accordingly.
Also, I saw this alloc isn't necessary either: cpus = kcalloc(nvec, sizeof(*cpus), GFP_KERNEL);
We can simply use the return from cpumask_local_spread() without saving all cpu numbers in a tmp array.
I will clean this up too :)
Thanks, - Haiyang