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, 21 Sep 2021 12:48:39 +0200
From:   Michal Hocko <mhocko@...e.com>
To:     Sultan Alsawaf <sultan@...neltoast.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        David Rientjes <rientjes@...gle.com>,
        Mel Gorman <mgorman@...e.de>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: Mark the OOM reaper thread as freezable

On Sat 18-09-21 16:39:20, Sultan Alsawaf wrote:
> From: Sultan Alsawaf <sultan@...neltoast.com>
> 
> The OOM reaper thread uses wait_event_freezable() without actually being
> marked as freezable. Fix it by adding a set_freezable() call.

After the follow up discussion it is clear what the patch does and why
it is needed. The changelog really begs for some clarification. I would
propose something like

"
The OOM reaper kthread uses wait_event_freezable while it is waiting for
any work. It is safe to freeze it while waiting. We, however, need to
prevent any activity after global freezer quiescent state because the
oom reaping is altering address space and this might alter the snapshot
theoretically (please note that this is mostly a theoretical concern not
being observed in practice so far) so the freezer has to wait for an
explicit freezing.

The current implementation doesn't work that way though because all
kernel threads are created with PF_NOFREEZE flag so they are
automatically excluded from freezing operation. This means that
oom_reaper can race with the system snapshoting if it was processing
while the system is being frozen. Fix that by set_freezable which will
make the oom_reaper visible to the freezer.
"

> 
> Fixes: aac453635549 ("mm, oom: introduce oom reaper")
> Signed-off-by: Sultan Alsawaf <sultan@...neltoast.com>

With the above or otherwise improved changelog feel free to add
Acked-by: Michal Hocko <mhocko@...e.com>

Thanks!

> ---
>  mm/oom_kill.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 831340e7ad8b..46a742b57735 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -641,6 +641,8 @@ static void oom_reap_task(struct task_struct *tsk)
>  
>  static int oom_reaper(void *unused)
>  {
> +	set_freezable();
> +
>  	while (true) {
>  		struct task_struct *tsk = NULL;
>  
> -- 
> 2.33.0

-- 
Michal Hocko
SUSE Labs

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ