Re: [PATCH] atomic: Fix bugs in 'fetch_or()' and rename it to 'xchg_or()'

From: Peter Zijlstra
Date: Tue Mar 15 2016 - 08:22:44 EST


On Tue, Mar 15, 2016 at 10:32:45AM +0100, Ingo Molnar wrote:
> +#ifndef xchg_or
> +# define xchg_or(ptr, mask) \
> +({ \
> + typeof(ptr) __ptr = (ptr); \
> + typeof(mask) __mask = (mask); \
> + \
> + typeof(*(__ptr)) __old, __val = *__ptr; \
> + \
> for (;;) { \
> + __old = cmpxchg(__ptr, __val, __val | __mask); \
> if (__old == __val) \
> break; \
> __val = __old; \
> } \
> + \
> __old; \
> })

As reported by you this explodes, and it obvious from the generated asm
why:

48e1: 89 c2 mov %eax,%edx
48e3: 41 89 d0 mov %edx,%r8d
48e6: 31 c9 xor %ecx,%ecx
48e8: 89 d0 mov %edx,%eax
48ea: 41 83 c8 08 or $0x8,%r8d
48ee: f0 44 0f b1 01 lock cmpxchg %r8d,(%rcx)
48f3: 39 c2 cmp %eax,%edx
48f5: 75 ea jne 48e1 <resched_curr+0x31>

That's an unconditional NULL deref.

What happens is that __ptr from xchg_or() aliasses with __ptr from
cmpxchg() and weird stuff happens.

If you do: s/__ptr/_ptr/ or similar on the xchg_or() code it all works
again.