Re: [PATCH] coredump: fix unfreezable coredumping task

From: Andrey Ryabinin
Date: Tue Nov 08 2016 - 11:13:01 EST


On 11/08/2016 01:26 AM, Andrew Morton wrote:
> On Fri, 30 Sep 2016 11:50:34 +0300 Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx> wrote:
>
>> It could be not possible to freeze coredumping task when it waits
>> for 'core_state->startup' completion, because threads are frozen
>> in get_signal() before they got a chance to complete 'core_state->startup'.
>>
>> Use freezer_do_not_count() to tell freezer to ignore coredumping
>> task while it waits for core_state->startup completion.
>>
>> Signed-off-by: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx
>
> The changelog provides no reason why this patch should be merged into
> -stable. Nor into anything else, really.
>
> Please (as always) provide a full description of the bug's end-user
> visible effects.
>

Sorry. Added the following paragraph:

Inability to freeze a task during suspend will cause suspend to fail.
Also CRIU uses cgroup freezer during dump operation. So with unfreezable
task CRIU dump will fail because it wait for transition from 'FREEZING'
to 'FROZEN' state which will never happen.


----
From: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx>
Subject: coredump: fix unfreezable coredumping task

It could be not possible to freeze coredumping task when it waits
for 'core_state->startup' completion, because threads are frozen
in get_signal() before they got a chance to complete 'core_state->startup'.

Inability to freeze a task during suspend will cause suspend to fail.
Also CRIU uses cgroup freezer during dump operation. So with unfreezable
task CRIU dump will fail because it wait for transition from 'FREEZING'
to 'FROZEN' state which will never happen.

Use freezer_do_not_count() to tell freezer to ignore coredumping
task while it waits for core_state->startup completion.

Link: http://lkml.kernel.org/r/1475225434-3753-1-git-send-email-aryabinin@xxxxxxxxxxxxx
Signed-off-by: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx>
Acked-by: Pavel Machek <pavel@xxxxxx>
Acked-by: Oleg Nesterov <oleg@xxxxxxxxxx>
Cc: Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>
Cc: Tejun Heo <tj@xxxxxxxxxx>
Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
Cc: Michal Hocko <mhocko@xxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
fs/coredump.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/fs/coredump.c b/fs/coredump.c
index 281b768..eb9c92c 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -1,6 +1,7 @@
#include <linux/slab.h>
#include <linux/file.h>
#include <linux/fdtable.h>
+#include <linux/freezer.h>
#include <linux/mm.h>
#include <linux/stat.h>
#include <linux/fcntl.h>
@@ -423,7 +424,9 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
if (core_waiters > 0) {
struct core_thread *ptr;

+ freezer_do_not_count();
wait_for_completion(&core_state->startup);
+ freezer_count();
/*
* Wait for all the threads to become inactive, so that
* all the thread context (extended register state, like
--