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

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

[Eduard Zateev]

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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=61464b62b688791cc24c
Reported-by: syzbot+a2e50fb40695811c2dab@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a2e50fb40695811c2dab
Signed-off-by: Eduard Zateev <hackerowskiy@gmail.com>
---
 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;
+}
+
 /*
  * Looks up an outgoing operation on a connection and returns a refcounted
  * pointer if found, or NULL otherwise.
@@ -766,18 +778,30 @@ int gb_operation_request_send(struct gb_operation *operation,
 	if (ret)
 		goto err_put;
 
+	/* Keep the operation alive while a fast completion races with send. */
+	gb_operation_get(operation);
 	ret = gb_message_send(operation->request, gfp);
 	if (ret)
-		goto err_put_active;
+		goto err_put_send;
 
 	if (timeout) {
 		operation->timer.expires = jiffies + msecs_to_jiffies(timeout);
 		add_timer(&operation->timer);
+
+		/*
+		 * Completion may have removed the timer before it was armed.
+		 * If so, remove the newly armed timer before the operation can
+		 * be released.
+		 */
+		if (!gb_operation_is_in_progress(operation))
+			timer_delete_sync(&operation->timer);
 	}
 
+	gb_operation_put(operation);
 	return 0;
 
-err_put_active:
+err_put_send:
+	gb_operation_put(operation);
 	gb_operation_put_active(operation);
 err_put:
 	gb_operation_put(operation);
-- 
2.55.0

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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=61464b62b688791cc24c
> Reported-by: syzbot+a2e50fb40695811c2dab@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a2e50fb40695811c2dab
> Signed-off-by: Eduard Zateev <hackerowskiy@gmail.com>
> ---
>  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