Re: [PATCH 3/3] ipmi:si: Fix check for a misbehaving BMC

Rafael J. Wysocki
2026-02-13T12:58:32Z

On Fri, Feb 13, 2026 at 7:55 AM Corey Minyard <corey@minyard.net> wrote:
>
> There is a race on checking the state in the sender, it needs to be
> checked under a lock. But you also need a check to avoid issues with
> a misbehaving BMC for run to completion mode. So leave the check at
> the beginning for run to completion, and add a check under the lock
> to avoid the race.
>
> Reported-by: Rafael J. Wysocki <rafael@kernel.org>
> Fixes: bc3a9d217755 ("ipmi:si: Gracefully handle if the BMC is non-functional")
> Signed-off-by: Corey Minyard <corey@minyard.net>

That's what I would do in sender(), more or less, so

Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

> ---
> drivers/char/ipmi/ipmi_si_intf.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 3667033fcc51..6eda61664aaa 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -924,9 +924,14 @@ static int sender(void *send_info, struct ipmi_smi_msg *msg)
> {
> struct smi_info *smi_info = send_info;
> unsigned long flags;
> + int rv = IPMI_CC_NO_ERROR;
>
> debug_timestamp(smi_info, "Enqueue");
>
> + /*
> + * Check here for run to completion mode. A check under lock is
> + * later.
> + */
> if (smi_info->si_state == SI_HOSED)
> return IPMI_BUS_ERR;
>
> @@ -940,18 +945,15 @@ static int sender(void *send_info, struct ipmi_smi_msg *msg)
> }
>
> spin_lock_irqsave(&smi_info->si_lock, flags);
> - /*
> - * The following two lines don't need to be under the lock for
> - * the lock's sake, but they do need SMP memory barriers to
> - * avoid getting things out of order. We are already claiming
> - * the lock, anyway, so just do it under the lock to avoid the
> - * ordering problem.
> - */
> - BUG_ON(smi_info->waiting_msg);
> - smi_info->waiting_msg = msg;
> - check_start_timer_thread(smi_info);
> + if (smi_info->si_state == SI_HOSED) {
> + rv = IPMI_BUS_ERR;
> + } else {
> + BUG_ON(smi_info->waiting_msg);
> + smi_info->waiting_msg = msg;
> + check_start_timer_thread(smi_info);
> + }
> spin_unlock_irqrestore(&smi_info->si_lock, flags);
> - return IPMI_CC_NO_ERROR;
> + return rv;
> }
>
> static void set_run_to_completion(void *send_info, bool i_run_to_completion)
> --
> 2.43.0
>