| 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
| ||
|
Message-ID: <BANLkTinaHki1oA4O3+FsoPDtFTLfqwRadA@mail.gmail.com>
Date: Mon, 23 May 2011 13:31:57 +0900
From: Minchan Kim <minchan.kim@...il.com>
To: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
akpm@...ux-foundation.org, caiqian@...hat.com, rientjes@...gle.com,
hughd@...gle.com, kamezawa.hiroyu@...fujitsu.com, oleg@...hat.com
Subject: Re: [PATCH 4/5] oom: don't kill random process
2011/5/20 KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>:
> CAI Qian reported oom-killer killed all system daemons in his
> system at first if he ran fork bomb as root. The problem is,
> current logic give them bonus of 3% of system ram. Example,
> he has 16GB machine, then root processes have ~500MB oom
> immune. It bring us crazy bad result. _all_ processes have
> oom-score=1 and then, oom killer ignore process memory usage
> and kill random process. This regression is caused by commit
> a63d83f427 (oom: badness heuristic rewrite).
>
> This patch changes select_bad_process() slightly. If oom points == 1,
> it's a sign that the system have only root privileged processes or
> similar. Thus, select_bad_process() calculate oom badness without
> root bonus and select eligible process.
>
> Also, this patch move finding sacrifice child logic into
> select_bad_process(). It's necessary to implement adequate
> no root bonus recalculation. and it makes good side effect,
> current logic doesn't behave as the doc.
>
> Documentation/sysctl/vm.txt says
>
> oom_kill_allocating_task
>
> If this is set to non-zero, the OOM killer simply kills the task that
> triggered the out-of-memory condition. This avoids the expensive
> tasklist scan.
>
> IOW, oom_kill_allocating_task shouldn't search sacrifice child.
> This patch also fixes this issue.
>
> Reported-by: CAI Qian <caiqian@...hat.com>
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> ---
> fs/proc/base.c | 2 +-
> include/linux/oom.h | 3 +-
> mm/oom_kill.c | 89 ++++++++++++++++++++++++++++----------------------
> 3 files changed, 53 insertions(+), 41 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index d6b0424..b608b69 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -482,7 +482,7 @@ static int proc_oom_score(struct task_struct *task, char *buffer)
>
> read_lock(&tasklist_lock);
> if (pid_alive(task)) {
> - points = oom_badness(task, NULL, NULL, totalpages);
> + points = oom_badness(task, NULL, NULL, totalpages, 1);
> ratio = points * 1000 / totalpages;
> }
> read_unlock(&tasklist_lock);
> diff --git a/include/linux/oom.h b/include/linux/oom.h
> index 0f5b588..3dd3669 100644
> --- a/include/linux/oom.h
> +++ b/include/linux/oom.h
> @@ -42,7 +42,8 @@ enum oom_constraint {
>
> /* The badness from the OOM killer */
> extern unsigned long oom_badness(struct task_struct *p, struct mem_cgroup *mem,
> - const nodemask_t *nodemask, unsigned long totalpages);
> + const nodemask_t *nodemask, unsigned long totalpages,
> + int protect_root);
> extern int try_set_zonelist_oom(struct zonelist *zonelist, gfp_t gfp_flags);
> extern void clear_zonelist_oom(struct zonelist *zonelist, gfp_t gfp_flags);
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 8bbc3df..7d280d4 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -133,7 +133,8 @@ static bool oom_unkillable_task(struct task_struct *p,
> * task consuming the most memory to avoid subsequent oom failures.
> */
> unsigned long oom_badness(struct task_struct *p, struct mem_cgroup *mem,
> - const nodemask_t *nodemask, unsigned long totalpages)
> + const nodemask_t *nodemask, unsigned long totalpages,
> + int protect_root)
> {
> unsigned long points;
> unsigned long score_adj = 0;
> @@ -186,7 +187,7 @@ unsigned long oom_badness(struct task_struct *p, struct mem_cgroup *mem,
> *
> * XXX: Too large bonus, example, if the system have tera-bytes memory..
> */
> - if (has_capability_noaudit(p, CAP_SYS_ADMIN)) {
> + if (protect_root && has_capability_noaudit(p, CAP_SYS_ADMIN)) {
> if (points >= totalpages / 32)
> points -= totalpages / 32;
> else
> @@ -298,8 +299,11 @@ static struct task_struct *select_bad_process(unsigned long *ppoints,
> {
> struct task_struct *g, *p;
> struct task_struct *chosen = NULL;
> - *ppoints = 0;
> + int protect_root = 1;
> + unsigned long chosen_points = 0;
> + struct task_struct *child;
>
> + retry:
> do_each_thread_reverse(g, p) {
> unsigned long points;
>
> @@ -332,7 +336,7 @@ static struct task_struct *select_bad_process(unsigned long *ppoints,
> */
> if (p == current) {
> chosen = p;
> - *ppoints = ULONG_MAX;
> + chosen_points = ULONG_MAX;
> } else {
> /*
> * If this task is not being ptraced on exit,
> @@ -345,13 +349,49 @@ static struct task_struct *select_bad_process(unsigned long *ppoints,
> }
> }
>
> - points = oom_badness(p, mem, nodemask, totalpages);
> - if (points > *ppoints) {
> + points = oom_badness(p, mem, nodemask, totalpages, protect_root);
> + if (points > chosen_points) {
> chosen = p;
> - *ppoints = points;
> + chosen_points = points;
> }
> } while_each_thread(g, p);
>
> + /*
> + * chosen_point==1 may be a sign that root privilege bonus is too large
> + * and we choose wrong task. Let's recalculate oom score without the
> + * dubious bonus.
> + */
> + if (protect_root && (chosen_points == 1)) {
> + protect_root = 0;
> + goto retry;
> + }
The idea is good to me.
But once we meet it, should we give up protecting root privileged processes?
How about decaying bonus point?
--
Kind regards,
Minchan Kim
--
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/
Powered by blists - more mailing lists