linux-kernel - Re: [pchecks v2 2/2] percpu: Add preemption checks to __this

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] [day] [month] [year] [list]

Message-ID: <0000014184818fb7-dc609dac-5ba0-4449-b24f-4e8bdfdac65b-000000@email.amazonses.com>
Date:	Fri, 4 Oct 2013 17:26:27 +0000
From:	Christoph Lameter <cl@...ux.com>
To:	Peter Zijlstra <peterz@...radead.org>
cc:	Tejun Heo <tj@...nel.org>, akpm@...uxfoundation.org,
	Steven Rostedt <srostedt@...hat.com>,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [pchecks v2 2/2] percpu: Add preemption checks to __this_cpu
 ops

On Fri, 4 Oct 2013, Peter Zijlstra wrote:

> > The __this_cpu_read_xxx() are asm primitives provided by various arches.
> > __this_cpu_read() is currently not overriden by any arches. That is why
> > the approach here of replicating only the higher level for raw_cpu_ops
> > works. Renaming the __this_cpu_xxx primitives would be a significant
> > change.
>
> This isn't about read; this is about all of them. Make sure the raw_*
> implementation is the actual real implementation; then implement the
> checking variant in terms of those.

Yes this is valid for all of them. The __this_cpu op is the low level
implementation.

> > > >  	if (!printk_ratelimit())
> > > >  		goto out_enable;
> > > >
> > > > -	printk(KERN_ERR "BUG: using smp_processor_id() in preemptible [%08x] "
> > > > -			"code: %s/%d\n",
> > > > +	printk(KERN_ERR "%s in preemptible [%08x] "
> > > > +			"code: %s/%d\n", what,
> > > >  			preempt_count() - 1, current->comm, current->pid);
> > >
> > > I would argue for keeping the "BUG" string intact and in front of the
> > > %s.
> >
> > Most of the place that I have seen are not bugs but there was a
> > reason for the code to run a __this_cpu op without preemption disabled.
>
> No; it is an actual BUG; it means that whoemever wrote the code didn't
> think straight and forgot to use the right primitive and comments.

??? The assumption needs to be that they used the function as it was
documented. No checks because they knew they were not needed in a
particular context.



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