linux-kernel - RE: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci

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, 23 Oct 2012 16:16:50 +0000
From:	"Luck, Tony" <tony.luck@...el.com>
To:	Tang Chen <tangchen@...fujitsu.com>,
	Borislav Petkov <bp@...en8.de>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"hpa@...or.com" <hpa@...or.com>,
	"miaox@...fujitsu.com" <miaox@...fujitsu.com>,
	"laijs@...fujitsu.com" <laijs@...fujitsu.com>,
	"wency@...fujitsu.com" <wency@...fujitsu.com>,
	"x86@...nel.org" <x86@...nel.org>,
	"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in
 cmci_rediscover().

> First of all, I do think I was answering your question. As I said
> before, if an online cpu == dying here, there must be something wrong.
> Am I right here ?

Yes - but there is a fuzzy line over where it is good to check for "something wrong"
or whether to trust that the caller of the function knew what they were doing.

For example we trust that "dying" is a valid cpu number.  If we were
super-paranoid that someone might change the code and call us with a
bad argument, we might add:

	BUG_ON(dying < 0 || dying >= MAX_NR_CPUS);

This would certainly help debug the case if someone did make a bogus
change ... but I think it is clear that this test is way past the fuzzy line and
into pointless.

Back to the case in question: do we think there is a credible case where
the "dying" cpu can show up in our "for_each_cpu_online()" loop? The
original author of the code was worried enough to make a test, but thought
that the appropriate action was to silently skip it. You want to add a WARN_ON,
which will cause users who read the console logs to worry, but that most users
will never see.

-Tony