[ERD][RAMEN9610-20347]ANDROID: Fix massive cpufreq_times memory leaks
authorSangkyu Kim <skwith.kim@samsung.com>
Fri, 27 Sep 2019 14:07:44 +0000 (23:07 +0900)
committerrobot <robot@samsung.com>
Mon, 7 Oct 2019 05:06:53 +0000 (14:06 +0900)
Every time _cpu_up() is called for a CPU, idle_thread_get() is called
which then re-initializes a CPU's idle thread that was already
previously created and cached in a global variable in
smpboot.c. idle_thread_get() calls init_idle() which then calls
__sched_fork(). __sched_fork() is where cpufreq_task_times_init() is,
and cpufreq_task_times_init() allocates memory for the task struct's
time_in_state array.

Since idle_thread_get() reuses a task struct instance that was already
previously created, this means that every time it calls init_idle(),
cpufreq_task_times_init() allocates this array again and overwrites
the existing allocation that the idle thread already had.

This causes memory to be leaked every time a CPU is onlined. In order
to fix this, move allocation of time_in_state into _do_fork to avoid
allocating it at all for idle threads. The cpufreq times interface is
intended to be used for tracking userspace tasks, so we can safely
remove it from the kernel's idle threads without killing any
functionality.

But that's not all!

Task structs can be freed outside of release_task(), which creates
another memory leak because a task struct can be freed without having
its cpufreq times allocation freed. To fix this, free the cpufreq
times allocation at the same time that task struct allocations are
freed, in free_task().

Since free_task() can also be called in error paths of copy_process()
after dup_task_struct(), set time_in_state to NULL immediately after
calling dup_task_struct() to avoid possible double free.

Bug description and fix adapted from patch submitted by
Sultan Alsawaf <sultanxda@gmail.com> at
https://android-review.googlesource.com/c/kernel/msm/+/700134

Bug: 110044919
Test: Hikey960 builds, boots & reports /proc/<pid>/time_in_state
correctly
Change-Id: I12fe7611fc88eb7f6c39f8f7629ad27b6ec4722c
Signed-off-by: Connor O'Brien <connoro@google.com>
kernel/exit.c
kernel/sched/core.c

index 67cc6f260ea5ba7d4ccf790f011e5245e7d49eea..644e280c68abfcac079a841a1a519bfeb8a28bf9 100644 (file)
@@ -187,9 +187,6 @@ void release_task(struct task_struct *p)
 {
        struct task_struct *leader;
        int zap_leader;
-#ifdef CONFIG_CPU_FREQ_TIMES
-       cpufreq_task_times_exit(p);
-#endif
 repeat:
        /* don't need to get the RCU readlock here - the process is dead and
         * can't be modifying its own credentials. But shut RCU-lockdep up */
index ac6c13bcda1295abe9a6c31ac9d259bf0dd2aa7a..75db2c6a63c731e19c16421529933b1dbf3e8542 100644 (file)
@@ -2246,10 +2246,6 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p)
        memset(&p->se.statistics, 0, sizeof(p->se.statistics));
 #endif
 
-#ifdef CONFIG_CPU_FREQ_TIMES
-       cpufreq_task_times_init(p);
-#endif
-
        RB_CLEAR_NODE(&p->dl.rb_node);
        init_dl_task_timer(&p->dl);
        init_dl_inactive_task_timer(&p->dl);