linux-kernel - Re: Possible Memory Leak in KCOV Linux 4.15-rc1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 21 Jan 2018 01:11:23 -0800
From:   Shankara Pailoor <sp3485@...umbia.edu>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        syzkaller <syzkaller@...glegroups.com>,
        Andrew Zhu Aday <andrew.aday@...umbia.edu>
Subject: Re: Possible Memory Leak in KCOV Linux 4.15-rc1

Hi Dmitry,

The leaks went away when I disabled and closed the old file
descriptors before opening new ones.

The patch you sent wouldn't work because t is not initialized at the
line. This seems to work for me

diff --git a/kernel/kcov.c b/kernel/kcov.c
index 7594c03..1397006 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -371,6 +371,8 @@ static int kcov_ioctl_locked(struct kcov *kcov,
unsigned int cmd,
                else
                        return -EINVAL;
                t = current;
+              if (!t->kcov)
+                       return -EBUSY;
                /* Cache in task struct for performance. */
                t->kcov_size = kcov->size;
                t->kcov_area = kcov->area;

On Sat, Jan 20, 2018 at 7:06 AM, Dmitry Vyukov <dvyukov@...gle.com> wrote:
> On Sat, Jan 20, 2018 at 4:01 PM, Shankara Pailoor <sp3485@...umbia.edu> wrote:
>> Hi Dmitry,
>>
>> I will try and get something to you tomorrow. Just wondering, but what
>> happens to the old struct kcov if a task opens /sys/kernel/debug/kcov
>> twice? I am looking here
>> https://elixir.free-electrons.com/linux/v4.15-rc8/source/kernel/kcov.c#L381
>> and I don't see where the previous struct would get freed.
>
> Good question. Perhaps we need something like:
>
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index 7594c033d98a..c76498018500 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -358,7 +358,7 @@ static int kcov_ioctl_locked(struct kcov *kcov,
> unsigned int cmd,
>                  */
>                 if (kcov->mode != KCOV_MODE_INIT || !kcov->area)
>                         return -EINVAL;
> -               if (kcov->t != NULL)
> +               if (kcov->t != NULL || t->kcov != NULL)
>                         return -EBUSY;
>                 if (arg == KCOV_TRACE_PC)
>                         kcov->mode = KCOV_MODE_TRACE_PC;
>
>
>
>> On Sat, Jan 20, 2018 at 4:38 AM, Dmitry Vyukov <dvyukov@...gle.com> wrote:
>>> On Fri, Jan 19, 2018 at 8:29 PM, Shankara Pailoor <sp3485@...umbia.edu> wrote:
>>>> Hi Dmitry,
>>>>
>>>> I added support for kcov in strace and I have been tracing a fairly
>>>> large program but after a little while, I notice that when I mmap a
>>>> new cover buffer, the call fails with ENOMEM. After killing the
>>>> program, I try and rerun and I notice that there is nearly no memory
>>>> on the system. When I do a kmemleak scan I get the following reports:
>>>>
>>>> I believe the problem occurs when I try and setup the kcov buffer
>>>> again after an exec. Instead of reusing the old file descriptor I open
>>>> kcov again within that process. In that case, I don't know what
>>>> happens to the old kcov struct.
>>>>
>>>> I don't see a maintainers list for kcov so I decided to email you
>>>> directly. Let me know what more information I can provide.
>>>
>>>
>>> Hi Shankara,
>>>
>>> Looks bad. Can you provide a reproducer?
>>> We extensively use kcov with syzkaller, but have not observed such
>>> leaks. Also I don't see anything obvious in the code.
>>>
>>> Thanks
>>