On Mon, 25 Jun 2018 00:05:21 +0900
Akinobu Mita <akinobu.mita@xxxxxxxxx> wrote:
> When the buffer is enabled for ina2xx driver, a dedicated kthread is
> invoked to capture mesurement data. When the buffer is disabled, the
> kthread is stopped.
>
> However if the kthread gets register access errors, it immediately exits
> and when the malfunctional buffer is disabled, the stale task_struct
> pointer is accessed as there is no kthread to be stopped.
>
> A similar issue in the usbip driver is prevented by kthread_get_run and
> kthread_stop_put helpers by increasing usage count of the task_struct.
> This change applies the same solution.
>
> Cc: Stefan Brüns <stefan.bruens@xxxxxxxxxxxxxx>
> Cc: Jonathan Cameron <jic23@xxxxxxxxxx>
> Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx>
Seems fine, but this is a fix so should have an appropriate fixes
tag. Feel free to send one in reply to this thread rather than a v2.
Without a fixes tag it can be very hard to know exactly where
a patch 'should' apply. I also have little visibility on
how important backporting htis issue is. What would actually trigger
the issue and is it likely to be seen in the wild?
Thanks,
Jonathan
> ---
> drivers/iio/adc/ina2xx-adc.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index 0635a79..d123962 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -30,6 +30,7 @@
> #include <linux/module.h>
> #include <linux/of_device.h>
> #include <linux/regmap.h>
> +#include <linux/sched/task.h>
> #include <linux/util_macros.h>
>
> #include <linux/platform_data/ina2xx.h>
> @@ -826,6 +827,7 @@ static int ina2xx_buffer_enable(struct iio_dev *indio_dev)
> {
> struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> unsigned int sampling_us = SAMPLING_PERIOD(chip);
> + struct task_struct *task;
>
> dev_dbg(&indio_dev->dev, "Enabling buffer w/ scan_mask %02x, freq = %d, avg =%u\n",
> (unsigned int)(*indio_dev->active_scan_mask),
> @@ -835,11 +837,17 @@ static int ina2xx_buffer_enable(struct iio_dev *indio_dev)
> dev_dbg(&indio_dev->dev, "Async readout mode: %d\n",
> chip->allow_async_readout);
>
> - chip->task = kthread_run(ina2xx_capture_thread, (void *)indio_dev,
> - "%s:%d-%uus", indio_dev->name, indio_dev->id,
> - sampling_us);
> + task = kthread_create(ina2xx_capture_thread, (void *)indio_dev,
> + "%s:%d-%uus", indio_dev->name, indio_dev->id,
> + sampling_us);
> + if (IS_ERR(task))
> + return PTR_ERR(task);
> +
> + get_task_struct(task);
> + wake_up_process(task);
> + chip->task = task;
>
> - return PTR_ERR_OR_ZERO(chip->task);
> + return 0;
> }
>
> static int ina2xx_buffer_disable(struct iio_dev *indio_dev)
> @@ -848,6 +856,7 @@ static int ina2xx_buffer_disable(struct iio_dev *indio_dev)
>
> if (chip->task) {
> kthread_stop(chip->task);
> + put_task_struct(chip->task);
> chip->task = NULL;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
