lkml.org 
[lkml]   [2019]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRE: [PATCH v2 4/4] PCI: hv: kmemleak: Track the page allocations for struct hv_pcibus_device
Date
From: Dexuan Cui <decui@microsoft.com> Sent: Tuesday, November 19, 2019 11:17 PM
>
> The page allocated for struct hv_pcibus_device contains pointers to other
> slab allocations in new_pcichild_device(). Since kmemleak does not track
> and scan page allocations, the slab objects will be reported as leaks
> (false positives). Use the kmemleak APIs to allow tracking of such pages.
>
> Reported-by: Lili Deng <v-lide@microsoft.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> This is actually v1. I use "v2" in the Subject just to be consistent with
> the other patches in the patchset.
>
> Without the patch, we can see the below warning in dmesg, if kmemleak is
> enabled:
>
> kmemleak: 1 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
>
> and "cat /sys/kernel/debug/kmemleak" shows:
>
> unreferenced object 0xffff9217d1f2bec0 (size 192):
> comm "kworker/u256:7", pid 100821, jiffies 4501481057 (age 61409.997s)
> hex dump (first 32 bytes):
> a8 60 b1 63 17 92 ff ff a8 60 b1 63 17 92 ff ff .`.c.....`.c....
> 02 00 00 00 00 00 00 00 80 92 cd 61 17 92 ff ff ...........a....
> backtrace:
> [<0000000006f7ae93>] pci_devices_present_work+0x326/0x5e0 [pci_hyperv]
> [<00000000278eb88a>] process_one_work+0x15f/0x360
> [<00000000c59501e6>] worker_thread+0x49/0x3c0
> [<000000000a0a7a94>] kthread+0xf8/0x130
> [<000000007075c2e7>] ret_from_fork+0x35/0x40
>
> drivers/pci/controller/pci-hyperv.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index d7e05d436b5e..cc73f49c9e03 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -46,6 +46,7 @@
> #include <asm/irqdomain.h>
> #include <asm/apic.h>
> #include <linux/irq.h>
> +#include <linux/kmemleak.h>
> #include <linux/msi.h>
> #include <linux/hyperv.h>
> #include <linux/refcount.h>
> @@ -2907,6 +2908,16 @@ static int hv_pci_probe(struct hv_device *hdev,
> hbus = (struct hv_pcibus_device *)get_zeroed_page(GFP_KERNEL);
> if (!hbus)
> return -ENOMEM;
> +
> + /*
> + * kmemleak doesn't track page allocations as they are not commonly
> + * used for kernel data structures, but here hbus->children indeed
> + * contains pointers to hv_pci_dev structs, which are dynamically
> + * created in new_pcichild_device(). Allow kmemleak to scan the page
> + * so we don't get a false warning of memory leak.
> + */
> + kmemleak_alloc(hbus, PAGE_SIZE, 1, GFP_KERNEL);
> +

As noted in the existing comments, the hbus data structure must not cross
a page boundary, so that a portion of it can be used as a hypercall argument.
Historically kzalloc() didn't provide any alignment guarantee, so
get_zeroed_page() is used. But a recent commit (59bb47985c1d) changes
the behavior of kmalloc()/kzalloc() so that alignment *is* guaranteed for
power of 2 allocation sizes. With this change, the better fix is to use
kzalloc() instead of get_zeroed_page(). Note that the allocation size should
be HV_HYP_PAGE_SIZE instead of PAGE_SIZE, as the hbus structure must not
cross a page boundary based on Hyper-V's concept of a page, which may be
different from the guest page size on some architectures.

Michael

> hbus->state = hv_pcibus_init;
>
> /*
> @@ -3133,6 +3144,8 @@ static int hv_pci_remove(struct hv_device *hdev)
>
> hv_put_dom_num(hbus->sysdata.domain);
>
> + /* Remove kmemleak object previously allocated in hv_pci_probe() */
> + kmemleak_free(hbus);
> free_page((unsigned long)hbus);
> return ret;
> }
> --
> 2.19.1

\
 
 \ /
  Last update: 2019-11-24 22:53    [W:0.129 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site