Commit d501d378 authored by William Breathitt Gray's avatar William Breathitt Gray
Browse files

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: f1d8a071 ("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: default avatarWilliam Breathitt Gray <william.gray@linaro.org>
parent d917a62a

drivers/counter/104-quad-8.c

+42 −22
Original line number Diff line number Diff line
@@ -232,34 +232,45 @@ static const enum counter_function quad8_count_functions_list[] = { 
	COUNTER_FUNCTION_QUADRATURE_X4,

};



static int quad8_function_read(struct counter_device *counter,

			       struct counter_count *count,

			       enum counter_function *function)

static int quad8_function_get(const struct quad8 *const priv, const size_t id,

			      enum counter_function *const function)

{

	struct quad8 *const priv = counter_priv(counter);

	const int id = count->id;

	unsigned long irqflags;



	spin_lock_irqsave(&priv->lock, irqflags);

	if (!priv->quadrature_mode[id]) {

		*function = COUNTER_FUNCTION_PULSE_DIRECTION;

		return 0;

	}



	if (priv->quadrature_mode[id])

	switch (priv->quadrature_scale[id]) {

	case 0:

		*function = COUNTER_FUNCTION_QUADRATURE_X1_A;

			break;

		return 0;

	case 1:

		*function = COUNTER_FUNCTION_QUADRATURE_X2_A;

			break;

		return 0;

	case 2:

		*function = COUNTER_FUNCTION_QUADRATURE_X4;

			break;

		return 0;

	default:

		/* should never reach this path */

		return -EINVAL;

	}

	else

		*function = COUNTER_FUNCTION_PULSE_DIRECTION;

}



static int quad8_function_read(struct counter_device *counter,

			       struct counter_count *count,

			       enum counter_function *function)

{

	struct quad8 *const priv = counter_priv(counter);

	unsigned long irqflags;

	int retval;



	spin_lock_irqsave(&priv->lock, irqflags);



	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