NAK: [PATCH][focal/linux-azure] PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer

[Tim Gardner]

I'll follow up with a proper SRU justification. Plus I forgot to attach 
the second patch.

On 5/17/22 07:20, Tim Gardner wrote:
> From: Dexuan Cui <decui at microsoft.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1973758
> 
> With the recent 59bb47985c1d ("mm, sl[aou]b: guarantee natural
> alignment for kmalloc(power-of-two)"), kzalloc() is able to allocate
> a 4KB buffer that is guaranteed to be 4KB-aligned. Here the size and
> alignment of hbus is important because hbus's field
> retarget_msi_interrupt_params must not cross a 4KB page boundary.
> 
> Here we prefer kzalloc to get_zeroed_page(), because a buffer
> allocated by the latter is not tracked and scanned by kmemleak, and
> hence kmemleak reports the pointer contained in the hbus buffer
> (i.e. the hpdev struct, which is created in new_pcichild_device() and
> is tracked by hbus->children) as memory leak (false positive).
> 
> If the kernel doesn't have 59bb47985c1d, get_zeroed_page() *must* be
> used to allocate the hbus buffer and we can avoid the kmemleak false
> positive by using kmemleak_alloc() and kmemleak_free() to ask
> kmemleak to track and scan the hbus buffer.
> 
> Reported-by: Lili Deng <v-lide at microsoft.com>
> Signed-off-by: Dexuan Cui <decui at microsoft.com>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi at arm.com>
> Reviewed-by: Michael Kelley <mikelley at microsoft.com>
> (backported from commit 877b911a5ba0733f62239055ee869f2e117b57da)
> [rtg - context adjustment]
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> ---
>   drivers/pci/controller/pci-hyperv.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 6272e7dda1fe..ea5aded65ec1 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -3388,7 +3388,7 @@ static int hv_pci_probe(struct hv_device *hdev,
>   	 * hv_pcibus_device contains the hypercall arguments for retargeting in
>   	 * hv_irq_unmask(). Those must not cross a page boundary.
>   	 */
> -	BUILD_BUG_ON(sizeof(*hbus) > PAGE_SIZE);
> +	BUILD_BUG_ON(sizeof(*hbus) > HV_HYP_PAGE_SIZE);
>   
>   	bridge = devm_pci_alloc_host_bridge(&hdev->device, 0);
>   	if (!bridge)
> @@ -3412,7 +3412,7 @@ static int hv_pci_probe(struct hv_device *hdev,
>   	 * positive by using kmemleak_alloc() and kmemleak_free() to ask
>   	 * kmemleak to track and scan the hbus buffer.
>   	 */
> -	hbus = kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> +	hbus = (struct hv_pcibus_device *)kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
>   	if (!hbus)
>   		return -ENOMEM;
>   
> @@ -3670,7 +3670,7 @@ static int hv_pci_remove(struct hv_device *hdev)
>   
>   	hv_put_dom_num(hbus->bridge->domain_nr);
>   
> -	free_page((unsigned long)hbus);
> +	kfree(hbus);
>   	return ret;
>   }
>   

-- 
-----------
Tim Gardner
Canonical, Inc