linux-kernel - Re: [RFC][PATCH 0/5] Introduce checks for preemptable code for 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: <alpine.DEB.2.00.1109211007450.12914@router.home>
Date:	Wed, 21 Sep 2011 10:16:51 -0500 (CDT)
From:	Christoph Lameter <cl@...two.org>
To:	Steven Rostedt <rostedt@...dmis.org>
cc:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	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>
Subject: Re: [RFC][PATCH 0/5] Introduce checks for preemptable code for
 this_cpu_read/write()

On Tue, 20 Sep 2011, Steven Rostedt wrote:

> BTW, Can you explain to me where the this_cpu_*() ops were designed to
> be used? The only places where "this_cpu_*()" is used in slub.c and
> page_alloc.c have irqs disabled on their use. I thought this was for
> slub and page_alloc?

Initially these were used for statistics that used per cpu counters. The
slub thing was an outgrow of this.

> Is this_cpu() made just for statistics? I see it used in the inode code
> for that, and some accounting in the namespace.c code.

That is the main use case yes.

> Note and there's places all over the kernel that uses this_cpu_read()
> and thinks preemption should be disabled. Just look at
> arch/x86/mm/tlb.c:
>
> 	/* Caller has disabled preemption */
> 	sender = this_cpu_read(tlb_vector_offset);
>
> Why the comment?
>
> My argument is that this_cpu_* is just confusing. Rename your use case
> and keep this_cpu_*() as what you want __this_cpu_*() to be.

Thought about this a bit last night. I think the main issue are these
this_cpu_read() and this_cpu_write() operations since people use those
irresponsibly. It usually does not make sense to read a value from a
random cpu nor does writing make sense. The situation is different for
per cpu counter increments where it does not matter which cpus counter is
incremented since we sum them up later anyways.

How about getting rid of this_cpu_read() and this_cpu_write() entirely?

Only allow __this_cpu_read and __this_cpu_write. There we check that the
caller has disabled preemption.

For the rare special cases (are there any?) that are legitimate use cases
for this_cpu_read/write we can use manual determination of per cpu
pointers and then just do a load via the pointer?

Or alternatively give this_cpu_read and write special names that make
their dangerousness clear.

In the case of slub there are only some this_cpu_write() things that can
be __this_cpu_write without a problem.

The __this_cpu_ptr() can become this_cpu_ptr as far as I can tell. This
should make it consistent so that we can check for disabled preemption for
all __this_cpu thingies.
--
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/