linux-kernel - [patch] oom: change warning for deprecated oom_adj to avoid WARN

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, 2 Aug 2011 02:54:01 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Andi Kleen <andi@...stfloor.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: [patch] oom: change warning for deprecated oom_adj to avoid
 WARN_ONCE()

On Mon, 1 Aug 2011, Andi Kleen wrote:

> Simply backtraces are not supposed to happen unless something 
> is really broken. That's not the case here. The old distribution
> works perfectly fine and will continue to do so.
> 

It won't work perfectly fine in a year when the tunable is removed 
completely and the code that was writing OOM_DISABLE to /proc/pid/oom_adj 
just fails and the task that was supposed to be prevented from being 
killed at all costs may now be killed.  The printk_once() over the past 
year didn't get that fixed up like the other applications I mentioned did, 
so we need to attract more attention.

> I still think reverting this patch is the right thing to do.
> 

Reverting the patch is ludicrous, otherwise there is little possibility 
that the remaining users of the deprecated interface will change if they 
haven't done so already.  I'm perfectly happy with changing it to a 
different style of warning other than using WARN_ONCE() like I've already 
said.  That doesn't require a revert.

I'm fine with this patch if Linus would like to apply it.

oom: change warning for deprecated oom_adj to avoid WARN_ONCE()

WARN_ONCE() emits a stack trace to the kernel log which leads userspace 
parsers to interpret it as being a serious error or malfunction within the 
kernel.  Change the warning to appear more like a lockdep warning while 
still trying to preserve the intention of be8f684d73d8 (oom: make 
deprecated use of oom_adj more verbose) to attract more attention to the 
use of a deprecated interface.

Reported-by: Andi Kleen <andi@...stfloor.org>
Signed-off-by: David Rientjes <rientjes@...gle.com>
---
 fs/proc/base.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1066,6 +1066,7 @@ static ssize_t oom_adjust_write(struct file *file, const char __user *buf,
 	char buffer[PROC_NUMBUF];
 	int oom_adjust;
 	unsigned long flags;
+	static bool warning_printed;
 	int err;

 	memset(buffer, 0, sizeof(buffer));
@@ -1118,9 +1119,15 @@ static ssize_t oom_adjust_write(struct file *file, const char __user *buf,
 	 * Warn that /proc/pid/oom_adj is deprecated, see
 	 * Documentation/feature-removal-schedule.txt.
 	 */
-	WARN_ONCE(1, "%s (%d): /proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead.\n",
-		  current->comm, task_pid_nr(current), task_pid_nr(task),
-		  task_pid_nr(task));
+	if (!warning_printed) {
+		warning_printed = true;
+		printk("\n===============================================================================\n");
+		printk("%s (%d): /proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead.\n",
+			current->comm, task_pid_nr(current), task_pid_nr(task),
+			task_pid_nr(task));
+		printk("===============================================================================\n\n");
+	}
+
 	task->signal->oom_adj = oom_adjust;
 	/*
 	 * Scale /proc/pid/oom_score_adj appropriately ensuring that a maximum
--
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/