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

[Eduard Zateev <hackerowskiy@gmail.com>]

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