lkml.org 
[lkml]   [2021]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] signal: Prevent sigqueue caching after task got released
Date
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. That's obviously bogus because nothing will free the cached
signal of the reaped group leader anymore.

Prevent this by setting tsk::sigqueue_cache to an error pointer value in
exit_task_sigqueue_cache().

Add comments to all relevant places.

Fixes: 4bad58ebc8bc ("signal: Allow tasks to cache one sigqueue struct")
Reported-by: syzbot+0bac5fec63d4f399ba98@syzkaller.appspotmail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
kernel/signal.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)

--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -435,6 +435,12 @@ static struct sigqueue *
* Preallocation does not hold sighand::siglock so it can't
* use the cache. The lockless caching requires that only
* one consumer and only one producer run at a time.
+ *
+ * For the regular allocation case it is sufficient to
+ * check @q for NULL because this code can only be called
+ * if the target task @t has not been reaped yet; which
+ * means this code can never observe the error pointer which is
+ * written to @t->sigqueue_cache in exit_task_sigqueue_cache().
*/
q = READ_ONCE(t->sigqueue_cache);
if (!q || sigqueue_flags)
@@ -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);
}

static void sigqueue_cache_or_free(struct sigqueue *q)
@@ -481,6 +492,10 @@ static void sigqueue_cache_or_free(struc
* is intentional when run without holding current->sighand->siglock,
* which is fine as current obviously cannot run __sigqueue_free()
* concurrently.
+ *
+ * The NULL check is safe even if current has been reaped already,
+ * in which case exit_task_sigqueue_cache() wrote an error pointer
+ * into current->sigqueue_cache.
*/
if (!READ_ONCE(current->sigqueue_cache))
WRITE_ONCE(current->sigqueue_cache, q);
\
 
 \ /
  Last update: 2021-06-22 01:09    [W:0.069 / U:0.368 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site