All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Qian Cai <cai@lca.pw>
Cc: tglx@linutronix.de, oleg@redhat.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next] posix-cpu-timers: fix memory leaks for task_struct
Date: Tue, 03 Mar 2020 23:15:43 -0600	[thread overview]
Message-ID: <87eeu8hf9c.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <20200304004336.960-1-cai@lca.pw> (Qian Cai's message of "Tue, 3 Mar 2020 19:43:36 -0500")

Qian Cai <cai@lca.pw> writes:

> The recent commit removed put_task_struct() in posix_cpu_timer_del()
> results in many memory leaks like this,

Good spotting but no.  The leak is in posix_cpu_timer_create.
There is a strong likely hood but no guarantee that the task
in posix_cpu_timer_del is the same as the task in
posix_cpu_timer_create.

Plus the point of it all is to use pid references instead of task
references.

Thank you very much for catching my braino.

Eric


> unreferenced object 0xc0000016d9b44480 (size 8192):
>   comm "timer_create01", pid 57749, jiffies 4295163733 (age 6159.670s)
>   hex dump (first 32 bytes):
>     02 00 00 00 00 00 00 00 10 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<0000000056aca129>] copy_process+0x26c/0x18e0
>     alloc_task_struct_node at kernel/fork.c:169
>     (inlined by) dup_task_struct at kernel/fork.c:877
>     (inlined by) copy_process at kernel/fork.c:1929
>     [<00000000bdbbf9f8>] _do_fork+0xac/0xb20
>     [<00000000dcb1c445>] __do_sys_clone+0x98/0xe0
>     __do_sys_clone at kernel/fork.c:2591
>     [<000000006c059205>] ppc_clone+0x8/0xc
>     ppc_clone at arch/powerpc/kernel/entry_64.S:479
>
> Fixes: 672ebe8eb017a5 ("posix-cpu-timers: Store a reference to a pid not a task")
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
>  kernel/time/posix-cpu-timers.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
> index afd1e959a282..e0b580deb61a 100644
> --- a/kernel/time/posix-cpu-timers.c
> +++ b/kernel/time/posix-cpu-timers.c
> @@ -446,8 +446,10 @@ static int posix_cpu_timer_del(struct k_itimer *timer)
>  
>  out:
>  	rcu_read_unlock();
> -	if (!ret)
> +	if (!ret) {
>  		put_pid(ctmr->pid);
> +		put_task_struct(p);
> +	}
>  
>  	return ret;
>  }

  reply	other threads:[~2020-03-04  5:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-04  0:43 [PATCH -next] posix-cpu-timers: fix memory leaks for task_struct Qian Cai
2020-03-04  5:15 ` Eric W. Biederman [this message]
2020-03-04  5:21 ` [PATCH timers/core] posix-cpu-timers: Put the task_struct in posix_cpu_timers_create Eric W. Biederman
2020-03-04  8:56   ` Thomas Gleixner
2020-03-04 14:32     ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87eeu8hf9c.fsf@x220.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=cai@lca.pw \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.