On Tue, 2016-12-20 at 20:03 +1100, Balbir Singh wrote:
>
> On 19/12/16 19:06, Madhavan Srinivasan wrote:
> > Move set_soft_enabled() from powerpc/kernel/irq.c to
> > asm/hw_irq.c, to force updates to paca-soft_enabled
> > done via these access function. Add "memory" clobber
> > to hint compiler since paca->soft_enabled memory is the target
> > here
> > +static inline notrace void soft_enabled_set(unsigned long enable)
> > +{
> > + __asm__ __volatile__("stb %0,%1(13)"
> > + : : "r" (enable), "i" (offsetof(struct paca_struct,
> > soft_enabled))
> > + : "memory");
> > +}
> > +
>
> Can't we just rewrite this in "C"?
>
> local_paca->soft_enabled = enable
Well, this is digging out another bloody can of baby eating worms...
So the reason we wrote it like that is because we had gcc playing
tricks to us.
It has been proved to move around references to local_paca, even
accross preempt_disable boundaries.
The above prevents it.
I'm about 100% sure we have a bunch of other issues related to PACA
access and gcc playing games though. I remember grepping the kernel
disassembly once for gcc doing funny things with r13 such as copying it
into another register or even saving and restoring it and my grep
didn't come up empty ...
Something we need to seriously look into one day. I have some ideas on
how to clean that up in the code to make hardening it easier, let's
talk next time Nick is in town.
Cheers,
Ben.
