Re: [PATCH] xen/x86: irq: Avoid a TOCTOU race in pirq_spin_lock_irq_desc()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.08.2020 10:53, Julien Grall wrote:
> Hi Jan,
> 
> On 18/08/2020 09:39, Jan Beulich wrote:
>> On 14.08.2020 21:25, Julien Grall wrote:
>>> Hi Andrew,
>>>
>>> Sorry for the late answer.
>>>
>>> On 23/07/2020 14:59, Andrew Cooper wrote:
>>>> On 23/07/2020 14:22, Julien Grall wrote:
>>>>> Hi Jan,
>>>>>
>>>>> On 23/07/2020 12:23, Jan Beulich wrote:
>>>>>> On 22.07.2020 18:53, Julien Grall wrote:
>>>>>>> --- a/xen/arch/x86/irq.c
>>>>>>> +++ b/xen/arch/x86/irq.c
>>>>>>> @@ -1187,7 +1187,7 @@ struct irq_desc *pirq_spin_lock_irq_desc(
>>>>>>>           for ( ; ; )
>>>>>>>         {
>>>>>>> -        int irq = pirq->arch.irq;
>>>>>>> +        int irq = read_atomic(&pirq->arch.irq);
>>>>>>
>>>>>> There we go - I'd be fine this way, but I'm pretty sure Andrew
>>>>>> would want this to be ACCESS_ONCE(). So I guess now is the time
>>>>>> to settle which one to prefer in new code (or which criteria
>>>>>> there are to prefer one over the other).
>>>>>
>>>>> I would prefer if we have a single way to force the compiler to do a
>>>>> single access (read/write).
>>>>
>>>> Unlikely to happen, I'd expect.
>>>>
>>>> But I would really like to get rid of (or at least rename)
>>>> read_atomic()/write_atomic() specifically because they've got nothing to
>>>> do with atomic_t's and the set of functionality who's namespace they share.
>>>
>>> Would you be happy if I rename both to READ_ONCE() and WRITE_ONCE()?
>>
>> Wouldn't this lead to confusion with Linux'es macros of the same names?
> 
> From my understanding, the purpose of READ_ONCE()/WRITE_ONCE() in Linux is 
> the same as our read_atomic()/write_atomic().
> 
> So I think it would be fine to rename them. An alternative would be port the 
> Linux version in Xen and drop ours.

The port of Linux'es {READ,WRITE}_ONCE() is our ACCESS_ONCE(). As pointed
out before, ACCESS_ONCE() and {read,write}_atomic() serve slightly
different purposes, and so far it looks like all of us are lacking ideas
on how to construct something that catches all cases by one single approach.

Jan