Re: [PATCH v2 1/2] x86/uaccess: rework user access speculative harden guards

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2025-01-04 01:20, Stefano Stabellini wrote:
> Hi Nicola, one question below
>
> On Wed, 27 Nov 2024, Nicola Vetrini wrote:
> > > #define AMD_OSVW_ERRATUM(osvw_id, ...)  osvw_id, __VA_ARGS__, 0
> > >
> > > where we're using the C99 form rather than the GNU extension, and where
> > > hence __VA_ARGS__ would - by extrapolation of the Misra rule - need
> > > parenthesizing, when it isn't and can't be.
> > >
> > > Isn't it rather the case that variable argument macros need a more general
> > > deviation, if not an adjustment to the Misra rule? Extending the Cc list
> > > some ...
> Nicola, if you look at the original patch:
> https://marc.info/?l=xen-devel&m=173261356716876
>
> "The current guards to select whether user accesses should be 
> speculative
> hardened violate Misra rule 20.7, as the UA_KEEP() macro doesn't (and 
> can't)
> parenthesize the 'args' argument."
>
> And the very first change in the patch is:
>
> diff --git a/xen/arch/x86/include/asm/uaccess.h 
> b/xen/arch/x86/include/asm/uaccess.h
> index 2d01669b96..6b8150ac22 100644
> --- a/xen/arch/x86/include/asm/uaccess.h
> +++ b/xen/arch/x86/include/asm/uaccess.h
> @@ -24,9 +24,6 @@ unsigned int copy_from_unsafe_ll(void *to, const void 
> *from, unsigned int n);
>  void noreturn __get_user_bad(void);
>  void noreturn __put_user_bad(void);
>
> -#define UA_KEEP(args...) args
> -#define UA_DROP(args...)
> -
>  /**
>   * get_guest: - Get a simple variable from guest space.
>   * @x:   Variable to store result.
>
>
> Do you think there is any way we could configure Eclair, with or 
> without
> a deviation, not to detect every use of UA_KEEP as violations?
I narrowed this violation down to a different treatment of the named 
variadic argument. Since the argument 'args' cannot be parenthesized as 
a regular argument could, the invocations of the 'UA_KEEP' cannot comply 
with the rule. Therefore, as an extension to the rule, ECLAIR currently 
ignores the use of '__VA_ARGS__' in a macro definition, but treats 
'args...' as a regular macro parameter name, hence the violation.
To be clear, these two definitions have the same semantics, but one 
shows a violation and the other doesn't
#define UA_KEEP(args...) args
#define UA_KEEP(...) __VA_ARGS__

I will update ECLAIR to treat the two forms as the same, so this patch 
can be dropped. If you think it's helpful I can send a patch spelling 
out this - arbitrary, but reasonable in my opinion - extension to the 
MISRA rule (which does not consider the implications related to the use 
of GNU exensions) so that contributors have a clear picture of the 
situation.
Thanks,
 Nicola

--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253