Re: [PATCH] greybus: operation: keep operation alive while arming timer

[Greg KH]

On Thu, Oct 22, 2026 at 03:48:17PM +0200, Eduard Zateev wrote:
> An operation can complete after gb_message_send() returns but before
> gb_operation_request_send() arms its timeout timer. The completion
> worker then sees no active timer, finishes the callback, and drops its
> operation reference. The request path may still arm the timer, so a later
> final put frees an operation with an active timer.
> 
> Take a temporary reference before sending and keep it through timer
> setup. After arming the timer, inspect the result under
> gb_operations_lock. If the operation has already completed,
> synchronously delete the timer before dropping the temporary reference.
> 
> OpenAI Codex assisted with the syzbot investigation, race analysis, patch,
> and changelog. The syzbot C reproducer drove Greybus ES2 under ARM64 QEMU
> but did not naturally trigger ODEBUG. In a test-only build, forcing
> completion work to finish before timer setup left the timer pending when
> the operation was destroyed before this change and inactive with this
> change.
> 
> Fixes: dbec27298b0d ("staging: greybus: operation: add generic timeout support")
> Reported-by: syzbot+61464b62b688791cc24c@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=61464b62b688791cc24c
> Reported-by: syzbot+a2e50fb40695811c2dab@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=a2e50fb40695811c2dab
> Signed-off-by: Eduard Zateev <hackerowskiy@xxxxxxxxx>
> ---
>  drivers/greybus/operation.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c
> index 7e12ffb2dd6..5bfee8638c9 100644
> --- a/drivers/greybus/operation.c
> +++ b/drivers/greybus/operation.c
> @@ -188,6 +188,18 @@ int gb_operation_result(struct gb_operation *operation)
>  }
>  EXPORT_SYMBOL_GPL(gb_operation_result);
>  
> +static bool gb_operation_is_in_progress(struct gb_operation *operation)
> +{
> +	unsigned long flags;
> +	bool in_progress;
> +
> +	spin_lock_irqsave(&gb_operations_lock, flags);
> +	in_progress = operation->errno == -EINPROGRESS;
> +	spin_unlock_irqrestore(&gb_operations_lock, flags);
> +
> +	return in_progress;

What prevents this value from changing right after you unlock and
return?  This isn't how to handle locking at all, right?

And how is syzbot testing greybus code?  What userspace / device is it
attempting to emulate here?

thanks,

greg k-h