Re: [PATCH for-4.22 v2 8/8] x86/mwait-idle: Add C-states validation

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, May 12, 2026 at 05:38:32PM +0200, Jan Beulich wrote:
> From: Artem Bityutskiy <artem.bityutskiy@xxxxxxxxxxxxxxx>
> 
> Add validation for C-states specified via the "table=" module parameter.
> Treat this module parameter as untrusted input and validate it thoroughly.
> 
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@xxxxxxxxxxxxxxx>
> Link: https://patch.msgid.link/20251216080402.156988-4-dedekind1@xxxxxxxxx
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
> be6a150829b3
> 
> Add __init to validate_cmdline_cstate(). Other adjustments to fit our env.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

One comment below.

> ---
> v2: Const-ify validate_cmdline_cstate()'s parameters.
> 
> --- a/xen/arch/x86/cpu/mwait-idle.c
> +++ b/xen/arch/x86/cpu/mwait-idle.c
> @@ -72,6 +72,11 @@ boolean_param("mwait-idle", opt_mwait_id
>  
>  /* The maximum allowed length for the 'table' module parameter  */
>  #define MAX_CMDLINE_TABLE_LEN 256
> +/* Maximum allowed C-state latency */
> +#define MAX_CMDLINE_LATENCY_US (5 * 1000 /* USEC_PER_MSEC */)
> +/* Maximum allowed C-state target residency */
> +#define MAX_CMDLINE_RESIDENCY_US (100 * 1000 /* USEC_PER_MSEC */)
> +
>  static char cmdline_table_str[MAX_CMDLINE_TABLE_LEN] __initdata;
>  string_param("mwait-idle.table", cmdline_table_str);
>  
> @@ -1590,6 +1595,41 @@ static char *__init get_cmdline_field(ch
>  }
>  
>  /**
> + * validate_cmdline_cstate - Validate a C-state from cmdline.
> + * @state: The C-state to validate.
> + * @prev_state: The previous C-state in the table or NULL.
> + *
> + * Return: 0 if the C-state is valid or -EINVAL otherwise.
> + */
> +static int __init validate_cmdline_cstate(const struct cpuidle_state *state,
> +                                       const struct cpuidle_state 
> *prev_state)
> +{
> +     if (state->exit_latency == 0)
> +             /* Exit latency 0 can only be used for the POLL state */
> +             return -EINVAL;
> +
> +     if (state->exit_latency > MAX_CMDLINE_LATENCY_US)
> +             return -EINVAL;
> +
> +     if (state->target_residency > MAX_CMDLINE_RESIDENCY_US)
> +             return -EINVAL;
> +
> +     if (state->target_residency < state->exit_latency)
> +             return -EINVAL;
> +
> +     if (!prev_state)
> +             return 0;
> +
> +     if (state->exit_latency <= prev_state->exit_latency)
> +             return -EINVAL;
> +
> +     if (state->target_residency <= prev_state->target_residency)
> +             return -EINVAL;
> +
> +     return 0;
> +}
> +
> +/**
>   * cmdline_table_adjust - Adjust the C-states table with data from cmdline.
>   *
>   * Adjust the C-states table with data from the 'mwait-idle.table' parameter
> @@ -1697,6 +1737,21 @@ static void __init cmdline_table_adjust(
>                      state->name, state->exit_latency, 
> state->target_residency);
>       }
>  
> +     /* Validate the adjusted C-states */
> +     for (i = 0; i < state_count; i++) {
> +             struct cpuidle_state *prev_state;

const?  FWIW, you could also init the field at definition, but I
understand this might diverge too much from the Linux code.

Thanks, Roger.