greybus: operation: fix broken activation logic

An operation should only be added to the connection active list if the
connection is in the enabled state, or if it is in the enabled_tx state
and the operation is not incoming.

This fixes a race where an early or late incoming request could be added
to the active list while the connection is being enabled or disabled,
something which could lead to use-after-free issues or worse.

Note that the early connection-state checks in the receive path
limited the impact of this bug.

Fixes: e903a2ce7379 ("connection: add unidirectional enabled state")
Reported-by: Alex Elder <elder@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c
index 31df413..b7cc59d 100644 (file)
--- a/drivers/staging/greybus/operation.c
+++ b/drivers/staging/greybus/operation.c
@@ -48,8 +48,8 @@ static int gb_operation_get_active(struct gb_operation *operation)
        spin_lock_irqsave(&connection->lock, flags);
 
        if (connection->state != GB_CONNECTION_STATE_ENABLED &&
        spin_lock_irqsave(&connection->lock, flags);
 
        if (connection->state != GB_CONNECTION_STATE_ENABLED &&

-                       connection->state != GB_CONNECTION_STATE_ENABLED_TX &&
-                       !gb_operation_is_incoming(operation)) {
+                       (connection->state != GB_CONNECTION_STATE_ENABLED_TX ||
+                        gb_operation_is_incoming(operation))) {

                spin_unlock_irqrestore(&connection->lock, flags);
                return -ENOTCONN;
        }
                spin_unlock_irqrestore(&connection->lock, flags);
                return -ENOTCONN;
        }