linux-kernel - Re: [PATCH -mm] Kill existing current task quickly

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]

Message-ID: <alpine.DEB.2.00.1002170128230.30931@chino.kir.corp.google.com>
Date:	Wed, 17 Feb 2010 01:36:33 -0800 (PST)
From:	David Rientjes <rientjes@...gle.com>
To:	Minchan Kim <minchan.kim@...il.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Nick Piggin <npiggin@...e.de>, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org
Subject: Re: [PATCH -mm] Kill existing current task quickly

On Wed, 17 Feb 2010, Minchan Kim wrote:

> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 3618be3..d5e3d70 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -32,6 +32,8 @@ int sysctl_panic_on_oom;
>  int sysctl_oom_kill_allocating_task;
>  int sysctl_oom_dump_tasks;
>  static DEFINE_SPINLOCK(zone_scan_lock);
> +
> +unsigned int nr_memdie; /* count of TIF_MEMDIE processes */
>  /* #define DEBUG */
> 
>  /*
> @@ -295,6 +297,8 @@ static struct task_struct
> *select_bad_process(unsigned long *ppoints,
> 
>                         chosen = p;
>                         *ppoints = ULONG_MAX;
> +                       if (nr_memdie == 0)
> +                               break;
>                 }
> 
>                 if (p->signal->oom_adj == OOM_DISABLE)

Nack, finding a candidate task with TIF_MEMDIE set is not the only time we 
return ERR_PTR(-1UL) from select_bad_process(): we also do it if any other 
task other than current is PF_EXITING.  Thus, we _must_ continue the 
tasklist scan to avoid needlessly killing current simply because it was 
the first PF_EXITING task in the tasklist.

> @@ -403,8 +407,6 @@ static void __oom_kill_task(struct task_struct *p,
> int verbose)
>                        K(p->mm->total_vm),
>                        K(get_mm_counter(p->mm, MM_ANONPAGES)),
>                        K(get_mm_counter(p->mm, MM_FILEPAGES)));
> -       task_unlock(p);
> -
>         /*
>          * We give our sacrificial lamb high priority and access to
>          * all the memory it needs. That way it should be able to
> @@ -412,7 +414,11 @@ static void __oom_kill_task(struct task_struct
> *p, int verbose)
>          */
>         p->rt.time_slice = HZ;
>         set_tsk_thread_flag(p, TIF_MEMDIE);
> -
> +       /*
> +        * nr_memdie is protected by task_lock.
> +        */
> +       nr_memdie++;
> +       task_unlock(p);
>         force_sig(SIGKILL, p);
>  }
> 

task_lock() is a per-task entity, i.e. each task_struct has an alloc_lock 
spinlock.  This cannot protect a global variable.
--
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/