Re: memory leak in alloc_huge_page

[Dmitry Vyukov]

On Tue, Dec 1, 2015 at 7:52 PM, Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote:
> On 12/01/2015 06:04 AM, Dmitry Vyukov wrote:
>> Hello,
>>
>> The following program leaks memory:
>>
>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>> #include <syscall.h>
>> #include <string.h>
>> #include <stdint.h>
>>
>> #define SYS_mlock2 325
>>
>> int main()
>> {
>>         syscall(SYS_mmap, 0x20000000ul, 0x1000ul, 0x3ul, 0x45031ul,
>> 0xfffffffffffffffful, 0x0ul);
>>         syscall(SYS_mlock2, 0x20000000ul, 0x1000ul, 0x1ul, 0, 0, 0);
>>         return 0;
>> }
>>
>> unreferenced object 0xffff88002eaafd88 (size 32):
>>   comm "a.out", pid 5063, jiffies 4295774645 (age 15.810s)
>>   hex dump (first 32 bytes):
>>     28 e9 4e 63 00 88 ff ff 28 e9 4e 63 00 88 ff ff  (.Nc....(.Nc....
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<     inline     >] kmalloc include/linux/slab.h:458
>>     [<ffffffff815efa64>] region_chg+0x2d4/0x6b0 mm/hugetlb.c:398
>>     [<ffffffff815f0c63>] __vma_reservation_common+0x2c3/0x390 mm/hugetlb.c:1791
>>     [<     inline     >] vma_needs_reservation mm/hugetlb.c:1813
>>     [<ffffffff815f658e>] alloc_huge_page+0x19e/0xc70 mm/hugetlb.c:1845
>>     [<     inline     >] hugetlb_no_page mm/hugetlb.c:3543
>>     [<ffffffff815fc561>] hugetlb_fault+0x7a1/0x1250 mm/hugetlb.c:3717
>>     [<ffffffff815fd349>] follow_hugetlb_page+0x339/0xc70 mm/hugetlb.c:3880
>>     [<ffffffff815a2bb2>] __get_user_pages+0x542/0xf30 mm/gup.c:497
>>     [<ffffffff815a400e>] populate_vma_page_range+0xde/0x110 mm/gup.c:919
>>     [<ffffffff815a4207>] __mm_populate+0x1c7/0x310 mm/gup.c:969
>>     [<ffffffff815b74f1>] do_mlock+0x291/0x360 mm/mlock.c:637
>>     [<     inline     >] SYSC_mlock2 mm/mlock.c:658
>>     [<ffffffff815b7a4b>] SyS_mlock2+0x4b/0x70 mm/mlock.c:648
>>
>> If this program run in a loop number of objects in kmalloc-32 slab
>> indeed grows infinitely.
>>
>> On commit 31ade3b83e1821da5fbb2f11b5b3d4ab2ec39db8 (Nov 29).
>>
>> There seems to be another leak if nrg is not NULL on this path, but
>> it's not what happens in my case since the WARNING does not fire.
>> Still something to fix:
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 827bb02..e97a31b 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -372,8 +372,10 @@ retry_locked:
>>                 spin_unlock(&resv->lock);
>>
>>                 trg = kmalloc(sizeof(*trg), GFP_KERNEL);
>> -               if (!trg)
>> +               if (!trg) {
>> +                       WARN_ON(nrg != NULL);
>>                         return -ENOMEM;
>> +               }
>>
>>                 spin_lock(&resv->lock);
>>                 list_add(&trg->link, &resv->region_cache);
>>
>>
>
> Thanks Dmitry,
>
> If nrg is not NULL, then it was added to the resv map and 'should' be
> free'ed when the map is free'ed.  This is not optimal, but I do not
> think it would lead to a leak.  I'll take a close look at this code
> with an emphasis on the leak you discovered.


Hi Mike,

Note that it's not just a leak report, it is an actual leak. You
should be able to reproduce it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/