All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: syzbot <syzbot+0bac5fec63d4f399ba98@syzkaller.appspotmail.com>,
	axboe@kernel.dk, christian@brauner.io, ebiederm@xmission.com,
	elver@google.com, linux-kernel@vger.kernel.org, pcc@google.com,
	peterz@infradead.org, syzkaller-bugs@googlegroups.com
Subject: Re: [PATCH] signal: Prevent sigqueue caching after task got released
Date: Tue, 22 Jun 2021 08:34:06 +0200	[thread overview]
Message-ID: <20210622063405.GA7570@redhat.com> (raw)
In-Reply-To: <878s32g6j5.ffs@nanos.tec.linutronix.de>

On 06/22, Thomas Gleixner wrote:
>
> syzbot reported a memory leak related to sigqueue caching. This happens
> when a thread group leader with child tasks is reaped.
>
> The group leader's sigqueue_cache is correctly freed. The group leader then
> reaps the child tasks and if any of them has a signal pending it caches
> that signal.

I guess you mean the race with exit_notify() ? Could you spell please?
I am just curious how exactly this problem was found.

This doesn't really matter, because damn yes, a task T can call
release_task(another_task)->sigqueue_cache_or_free() after
exit_task_sigqueue_cache(T) was already called. For example, a last non-leader
thread exits and reaps a zombie leader.

Somehow I thought that exit_task_sigqueue_cache() at the end of __exit_signal()
should fix this problem, but this is obviously wrong.


> @@ -463,13 +469,18 @@ void exit_task_sigqueue_cache(struct tas
>  	struct sigqueue *q = tsk->sigqueue_cache;
>  
>  	if (q) {
> -		tsk->sigqueue_cache = NULL;
>  		/*
>  		 * Hand it back to the cache as the task might
>  		 * be self reaping which would leak the object.
>  		 */
>  		 kmem_cache_free(sigqueue_cachep, q);
>  	}
> +
> +	/*
> +	 * Set an error pointer to ensure that @tsk will not cache a
> +	 * sigqueue when it is reaping it's child tasks
> +	 */
> +	tsk->sigqueue_cache = ERR_PTR(-1);
>  }


Reviewed-by: Oleg Nesterov <oleg@redhat.com>


  reply	other threads:[~2021-06-22  6:34 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-06 14:32 [syzbot] memory leak in __send_signal syzbot
2021-06-21 23:08 ` [PATCH] signal: Prevent sigqueue caching after task got released Thomas Gleixner
2021-06-22  6:34   ` Oleg Nesterov [this message]
2021-06-22  7:59     ` Thomas Gleixner
2021-06-22  8:06   ` Christian Brauner
2021-06-22 13:59   ` [tip: core/urgent] " tip-bot2 for Thomas Gleixner

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=20210622063405.GA7570@redhat.com \
    --to=oleg@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=christian@brauner.io \
    --cc=ebiederm@xmission.com \
    --cc=elver@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pcc@google.com \
    --cc=peterz@infradead.org \
    --cc=syzbot+0bac5fec63d4f399ba98@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.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.