Re: [PATCH v6 2/4] xen: common: add ability to enable stack protector

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.02.2025 03:49, Volodymyr Babchuk wrote:> --- /dev/null
> +++ b/xen/include/xen/stack-protector.h
> @@ -0,0 +1,43 @@
> +#ifndef __XEN_STACK_PROTECTOR_H__
> +#define __XEN_STACK_PROTECTOR_H__
> +
> +#ifdef CONFIG_STACK_PROTECTOR
> +
> +extern unsigned long __stack_chk_guard;
> +
> +/*
> + * This function should be called from a C function that escapes stack
> + * canary tracking (by calling reset_stack_and_jump() for example).
> + */
> +static inline void boot_stack_chk_guard_setup(void)

As was requested in v5 review, this needs to be always_inline. You cannot
chance the compiler deciding to make an out-of-line function.

> +{
> +    /*
> +     * Linear congruent generator (X_n+1 = X_n * a + c).
> +     *
> +     * Constant is taken from "Tables Of Linear Congruential
> +     * Generators Of Different Sizes And Good Lattice Structure" by
> +     * Pierre L’Ecuyer.
> +     */
> +#if BITS_PER_LONG == 32
> +    const unsigned long a = 2891336453UL;
> +#else
> +    const unsigned long a = 2862933555777941757UL;
> +#endif
> +    const unsigned long c = 1;
> +
> +    unsigned long cycles = get_cycles();
> +
> +    /* Use the initial value if we can't generate random one */
> +    if ( !cycles )
> +        return;
> +
> +    __stack_chk_guard = cycles * a + c;
> +}
> +
> +#else
> +
> +static inline void boot_stack_chk_guard_setup(void) {};
> +
> +#endif

Overall I think it would be neater (less redundancy) if the #ifdef lived inside
the body of the function. Having the decl of __stack_chk_guard always visible
isn't a major concern, imo.

Jan