On 14.03.2024 23:59, Stefano Stabellini wrote: > On Mon, 11 Mar 2024, Simone Ballarin wrote: >> On 11/03/24 14:56, Jan Beulich wrote: >>> On 11.03.2024 13:00, Simone Ballarin wrote: >>>> On 11/03/24 11:08, Jan Beulich wrote: >>>>> On 11.03.2024 09:59, Simone Ballarin wrote: >>>>>> --- a/xen/arch/arm/include/asm/hypercall.h >>>>>> +++ b/xen/arch/arm/include/asm/hypercall.h >>>>>> @@ -1,3 +1,4 @@ >>>>>> +/* SAF-5-safe direct inclusion guard before */ >>>>>> #ifndef __XEN_HYPERCALL_H__ >>>>>> #error "asm/hypercall.h should not be included directly - include >>>>>> xen/hypercall.h instead" >>>>>> #endif >>>>>> --- a/xen/arch/x86/include/asm/hypercall.h >>>>>> +++ b/xen/arch/x86/include/asm/hypercall.h >>>>>> @@ -2,6 +2,7 @@ >>>>>> * asm-x86/hypercall.h >>>>>> */ >>>>>> +/* SAF-5-safe direct inclusion guard before */ >>>>>> #ifndef __XEN_HYPERCALL_H__ >>>>>> #error "asm/hypercall.h should not be included directly - include >>>>>> xen/hypercall.h instead" >>>>>> #endif >>>>> >>>>> Iirc it was said that this way checking for correct guards is suppressed >>>>> altogether in Eclair, which is not what we want. Can you clarify this, >>>>> please? >>>>> >>>> >>>> My first change was moving this check inside the guard. >>>> You commented my patch saying that this would be an error because someone >>>> can >>>> include it directly if it has already been included indirectly. >>>> I replied telling that this was the case also before the change. >>>> You agreed with me, and we decided that the correct thing would be fixing >>>> the >>>> check and not apply my temporary change to address the finding. >>>> >>>> Considering that the code should be amended, a SAF deviation seems to me >>>> the most appropriate way for suppressing these findings. >>> >>> Since I don't feel your reply addresses my question, asking differently: >>> With >>> your change in place, will failure to have proper guards (later) in these >>> headers still be reported by Eclair? >> >> No, if you put something between the check and the guard, >> no violation will be reported. > > From this email exchange I cannot under if Jan is OK with this patch or > not.
Whether I'm okay(ish) with the patch here depends on our position towards the lost checking in Eclair mentioned above. To me it still looks relevant that checking for a guard occurs, even if that isn't first in a file for some specific reason. Jan
