Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1280895 > unrolled thread

memory leak in alloc_huge_page

Started byDmitry Vyukov <dvyukov@google.com>
First post2015-12-01 15:10 +0100
Last post2015-12-02 02:00 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

Contents

  memory leak in alloc_huge_page Dmitry Vyukov <dvyukov@google.com> - 2015-12-01 15:10 +0100
    Re: memory leak in alloc_huge_page Mike Kravetz <mike.kravetz@oracle.com> - 2015-12-01 20:00 +0100
      Re: memory leak in alloc_huge_page Dmitry Vyukov <dvyukov@google.com> - 2015-12-01 20:50 +0100
        Re: memory leak in alloc_huge_page Mike Kravetz <mike.kravetz@oracle.com> - 2015-12-02 02:00 +0100

#1280895 — memory leak in alloc_huge_page

FromDmitry Vyukov <dvyukov@google.com>
Date2015-12-01 15:10 +0100
Subjectmemory leak in alloc_huge_page
Message-ID<qATN0-2eD-15@gated-at.bofh.it>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1281119

FromMike Kravetz <mike.kravetz@oracle.com>
Date2015-12-01 20:00 +0100
Message-ID<qAYjE-4To-3@gated-at.bofh.it>
In reply to#1280895
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.

-- 
Mike Kravetz

> Thanks
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281159

FromDmitry Vyukov <dvyukov@google.com>
Date2015-12-01 20:50 +0100
Message-ID<qAZ62-5u6-19@gated-at.bofh.it>
In reply to#1281119
On Tue, Dec 1, 2015 at 7:52 PM, Mike Kravetz <mike.kravetz@oracle.com> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281378

FromMike Kravetz <mike.kravetz@oracle.com>
Date2015-12-02 02:00 +0100
Message-ID<qB3W2-8uw-3@gated-at.bofh.it>
In reply to#1281159
On 12/01/2015 11:45 AM, Dmitry Vyukov wrote:
> On Tue, Dec 1, 2015 at 7:52 PM, Mike Kravetz <mike.kravetz@oracle.com> wrote:
>> On 12/01/2015 06:04 AM, Dmitry Vyukov wrote:

>>> 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.
>>
>> 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.
> 

OK, finally found the bug which is in region_del().  It does not correctly
handle a "placeholder" region descriptor that is left after an aborted
operation when the start of the region to be deleted is for the same page.

I will have a patch shortly, after some testing.

-- 
Mike Kravetz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web