[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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).

And this is of course besides the fact that I think we have many
more instances where guaranteeing a single access would be
needed, if we're afraid of the described permitted compiler
behavior. Which then makes me wonder if this is really something
we should fix one by one, rather than by at least larger scope
audits (in order to not suggest "throughout the code base").

As a minor remark, unless you've observed problematic behavior,
would you mind adding "potential" or "theoretical" to the title?




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.