linux-kernel - Re: [RFC][PATCH 2/5] mm: Switch mod_state() to __this_cpu

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: <1316476089.29966.9.camel@gandalf.stny.rr.com>
Date:	Mon, 19 Sep 2011 19:48:09 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Christoph Lameter <cl@...two.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Christoph Lameter <cl@...ux.com>
Subject: Re: [RFC][PATCH 2/5] mm: Switch mod_state() to __this_cpu_read()

On Mon, 2011-09-19 at 17:02 -0500, Christoph Lameter wrote:
> On Mon, 19 Sep 2011, Steven Rostedt wrote:
> 
> > From: Steven Rostedt <srostedt@...hat.com>
> >
> > The code in mod_state() is already made to handle the raciness of
> > this_cpu_read(). Have the code use __this_cpu_read() instead so
> > the debug code does not trigger warnings about it.
> 
> Why would there be a warning triggered? this_cpu_read should take care of
> disabling preemption for the read if needed. In fact the fallback case
> does do exactly that.

What does disabling preemption for just reading the cpu variable help
anywhere? Once you read the variable, and enable preemption, you can
migrate. Then that info you have could be useless.

> 
> I think it would make more sence if __this_cpu_read() could be made to
> trigger a warning if used in context where preemption could be off.

Hmm, maybe I should have called it raw_this_cpu_read() then it would
probably have better meaning. Just like smp_processor_id() and
raw_smp_processor_id().

The point is, most places use this_cpu_read(). Adding a "__" or "raw_"
prefix usually means that less checks are done. Think of the
copy_from_user(). You have __copy_from_user() that does less checks.

Most of the per_cpu() and get_cpu_var() code was blindly replaced with
the this_cpu_*() variants. The original code had preemption disabled
checks. Now they don't. Which means if you use the this_cpu_read() and
then make some decision on it, that read may be useless and buggy.

We have smp_processor_id() that does the check (and the old per_cpu()
and get_cpu_var()). Now we have this_cpu_read() which replaced most of
the per_cpu() code in the kernel, making them vulnerable to bugs when
preemption is enabled.

-- Steve

--
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/