linux-kernel - Re: [Patch v2] freezer: check OOM kill while being frozen

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 12 Aug 2014 17:53:33 -0700
From:	Cong Wang <xiyou.wangcong@...il.com>
To:	Michal Hocko <mhocko@...e.cz>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	David Rientjes <rientjes@...gle.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>, Tejun Heo <tj@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [Patch v2] freezer: check OOM kill while being frozen

On Tue, Aug 12, 2014 at 4:44 AM, Michal Hocko <mhocko@...e.cz> wrote:
> Has it stopped working as a result of a3201227f803 (freezer: make
> freezing() test freeze conditions in effect instead of TIF_FREEZE)?
> It removes clear_freeze_flag from __thaw_task and so the OOM victim
> cannot escape from the refrigerator. But there were a lot of changes in
> that area at the time so I might be missing some subtle details.

Probably, I don't have time to dig the history.

>
>> __thaw_task() just checks if it's frozen and then wakes it up,
>> but the frozen task, after waking up, will check if freezing()
>> is still true and continue to freeze itself if so. __thaw_task()
>> can't make freezing() return false since it doesn't change any
>> of these conditions, especially cgroup_freezing().
>>
>> Fix this straightly by checking if the frozen process itself
>> has been killed by OOM killer, so that the frozen process will
>> recover itself and be killed finally.
>
> OK, this should work. I remember Tejun mentioned that he wanted frozen
> tasks to be killable but I have no idea where this went. Maybe we want
> to go with the OOM part now as it fixes a real bug.


I think he rejected the patch from Bart which makes frozen tasks killable.

>> +static bool i_should_thaw_myself(bool check_kthr_stop)
>
> should_thaw_current?
>

I have no strong preference, whatever you prefer...

>> @@ -147,12 +156,6 @@ void __thaw_task(struct task_struct *p)
>>  {
>>       unsigned long flags;
>>
>> -     /*
>> -      * Clear freezing and kick @p if FROZEN.  Clearing is guaranteed to
>> -      * be visible to @p as waking up implies wmb.  Waking up inside
>> -      * freezer_lock also prevents wakeups from leaking outside
>> -      * refrigerator.
>> -      */
>
> This is an unrelated change.
>

It is, because as I explained, __thaw_task() doesn't do what this comment
says therefore needs this patch. Or you prefer to make it a separated patch?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/