Re: ptep_establish/establish_pte needs set_pte_atomic and all set_pte must be written in asm

[Andrea Arcangeli <andrea@novell.com>]

On Sat, Sep 25, 2004 at 08:31:13PM -0400, Rik van Riel wrote:
> On Sun, 26 Sep 2004, Andrea Arcangeli wrote:
> 
> > But even ppc64 is wrong as far as C is concerned,
> 
> Looks fine to me.  From include/asm-ppc64/pgtable.h
> 
> static inline void set_pte(pte_t *ptep, pte_t pte)
> {
>         if (pte_present(*ptep)) {
>                 pte_clear(ptep);
>                 flush_tlb_pending();
>         }
>         *ptep = __pte(pte_val(pte)) & ~_PAGE_HPTEFLAGS;
> }

As far as the C language is concerned that *ptep = something can be
implemented with 8 writes of 1 byte each (or alternatively with an
assembler instruction that may make the written data visible not
atomically to other cpus, despite it was written with a single opcode,
similarly to what happens if you use incl without the lock prefix). I'm
not saying such instruction exists in ppc64, but the compiler is
definitely allowed to break the above. You can blame on the compiler to
be inefficient, but you can't blame on the compiler for the security
hazard it would generate. Only the kernel would be to blame if for
whatever reason a gcc version would be underoptimized.

I perfectly know in practice the above is "almost guaranteed" to work,
it's just the "almost" that's not good enough for me ;)

anyways this is just a corollary to the x86 true bug, where a smp_wmb()
sits in between the two separate writes, which makes the x86 set_pte
obviously not atomic even in practice (not just in theory like for
ppc64 and all other archs). I thought it was better to fix the
theoretical bugs too, and they are true bugs as far as the C language is
concerned, even if they're not triggering with the current gcc
implementation of the language (and with any other decent compiler ;).