counter: 104-quad-8: Fix race getting function mode and direction

The quad8_action_read() function checks the Count function mode and
Count direction without first acquiring a lock. This is a race condition
because the function mode could change by the time the direction is
checked.

Because the quad8_function_read() already acquires a lock internally,
the quad8_function_read() is refactored to spin out the no-lock code to
a new quad8_function_get() function.

To resolve the race condition in quad8_action_read(), a lock is acquired
before calling quad8_function_get() and quad8_direction_read() in order
to get both function mode and direction atomically.

Fixes: f1d8a071d45b ("counter: 104-quad-8: Add Generic Counter interface support")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20221020141121.15434-1-william.gray@linaro.org/
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>

diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c
index 77a863b7eefeac1a3fbd1d005e57eb508aadb604..deed4afadb298e1173a55a729a1708c1e298994c 100644 (file)
--- a/drivers/counter/104-quad-8.c
+++ b/drivers/counter/104-quad-8.c
@@ -232,34 +232,45 @@ static const enum counter_function quad8_count_functions_list[] = {
        COUNTER_FUNCTION_QUADRATURE_X4,
 };
 
+static int quad8_function_get(const struct quad8 *const priv, const size_t id,
+                             enum counter_function *const function)
+{
+       if (!priv->quadrature_mode[id]) {
+               *function = COUNTER_FUNCTION_PULSE_DIRECTION;
+               return 0;
+       }
+
+       switch (priv->quadrature_scale[id]) {
+       case 0:
+               *function = COUNTER_FUNCTION_QUADRATURE_X1_A;
+               return 0;
+       case 1:
+               *function = COUNTER_FUNCTION_QUADRATURE_X2_A;
+               return 0;
+       case 2:
+               *function = COUNTER_FUNCTION_QUADRATURE_X4;
+               return 0;
+       default:
+               /* should never reach this path */
+               return -EINVAL;
+       }
+}
+
 static int quad8_function_read(struct counter_device *counter,
                               struct counter_count *count,
                               enum counter_function *function)
 {
        struct quad8 *const priv = counter_priv(counter);
-       const int id = count->id;
        unsigned long irqflags;
+       int retval;
 
        spin_lock_irqsave(&priv->lock, irqflags);
 
-       if (priv->quadrature_mode[id])
-               switch (priv->quadrature_scale[id]) {
-               case 0:
-                       *function = COUNTER_FUNCTION_QUADRATURE_X1_A;
-                       break;
-               case 1:
-                       *function = COUNTER_FUNCTION_QUADRATURE_X2_A;
-                       break;
-               case 2:
-                       *function = COUNTER_FUNCTION_QUADRATURE_X4;
-                       break;
-               }
-       else
-               *function = COUNTER_FUNCTION_PULSE_DIRECTION;
+       retval = quad8_function_get(priv, count->id, function);
 
        spin_unlock_irqrestore(&priv->lock, irqflags);
 
-       return 0;
+       return retval;
 }
 
 static int quad8_function_write(struct counter_device *counter,
@@ -359,6 +370,7 @@ static int quad8_action_read(struct counter_device *counter,
                             enum counter_synapse_action *action)
 {
        struct quad8 *const priv = counter_priv(counter);
+       unsigned long irqflags;
        int err;
        enum counter_function function;
        const size_t signal_a_id = count->synapses[0].signal->id;
@@ -374,9 +386,21 @@ static int quad8_action_read(struct counter_device *counter,
                return 0;
        }
 
-       err = quad8_function_read(counter, count, &function);
-       if (err)
+       spin_lock_irqsave(&priv->lock, irqflags);
+
+       /* Get Count function and direction atomically */
+       err = quad8_function_get(priv, count->id, &function);
+       if (err) {
+               spin_unlock_irqrestore(&priv->lock, irqflags);
+               return err;
+       }
+       err = quad8_direction_read(counter, count, &direction);
+       if (err) {
+               spin_unlock_irqrestore(&priv->lock, irqflags);
                return err;
+       }
+
+       spin_unlock_irqrestore(&priv->lock, irqflags);
 
        /* Default action mode */
        *action = COUNTER_SYNAPSE_ACTION_NONE;
@@ -389,10 +413,6 @@ static int quad8_action_read(struct counter_device *counter,
                return 0;
        case COUNTER_FUNCTION_QUADRATURE_X1_A:
                if (synapse->signal->id == signal_a_id) {
-                       err = quad8_direction_read(counter, count, &direction);
-                       if (err)
-                               return err;
-
                        if (direction == COUNTER_COUNT_DIRECTION_FORWARD)
                                *action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
                        else