linux-kernel - Re: [PATCH 1/5] iio: adc: spear_adc: Replace indio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210221163723.53d8c7a8@archlinux>
Date:   Sun, 21 Feb 2021 16:37:23 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Mircea Caprioru <mircea.caprioru@...log.com>
Cc:     <Michael.Hennerich@...log.com>, <alexandru.ardelean@...log.com>,
        <lars@...afoo.de>, <gregkh@...uxfoundation.org>,
        <linux-kernel@...r.kernel.org>, <linux-iio@...r.kernel.org>,
        Sergiu Cuciurean <sergiu.cuciurean@...log.com>
Subject: Re: [PATCH 1/5] iio: adc: spear_adc: Replace indio_dev->mlock with
 own device lock

On Mon, 28 Sep 2020 16:13:29 +0300
Mircea Caprioru <mircea.caprioru@...log.com> wrote:

> From: Sergiu Cuciurean <sergiu.cuciurean@...log.com>
> 
> As part of the general cleanup of indio_dev->mlock, this change replaces
> it with a local lock on the device's state structure.
> 
> This is part of a bigger cleanup.
> Link: https://lore.kernel.org/linux-iio/CA+U=Dsoo6YABe5ODLp+eFNPGFDjk5ZeQEceGkqjxXcVEhLWubw@mail.gmail.com/
> 
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@...log.com>
> Signed-off-by: Mircea Caprioru <mircea.caprioru@...log.com>

I guess I was waiting for a v2 of the series.  Seeing as it has been a while
and the first 3 patches are fine on their own, I'll pick them up now.

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/spear_adc.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/spear_adc.c b/drivers/iio/adc/spear_adc.c
> index 1bc986a7009d..d93e580b3dc5 100644
> --- a/drivers/iio/adc/spear_adc.c
> +++ b/drivers/iio/adc/spear_adc.c
> @@ -75,6 +75,15 @@ struct spear_adc_state {
>  	struct adc_regs_spear6xx __iomem *adc_base_spear6xx;
>  	struct clk *clk;
>  	struct completion completion;
> +	/*
> +	 * Lock to protect the device state during a potential concurrent
> +	 * read access from userspace. Reading a raw value requires a sequence
> +	 * of register writes, then a wait for a completion callback,
> +	 * and finally a register read, during which userspace could issue
> +	 * another read request. This lock protects a read access from
> +	 * ocurring before another one has finished.
> +	 */
> +	struct mutex lock;
>  	u32 current_clk;
>  	u32 sampling_freq;
>  	u32 avg_samples;
> @@ -146,7 +155,7 @@ static int spear_adc_read_raw(struct iio_dev *indio_dev,
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		mutex_lock(&indio_dev->mlock);
> +		mutex_lock(&st->lock);
>  
>  		status = SPEAR_ADC_STATUS_CHANNEL_NUM(chan->channel) |
>  			SPEAR_ADC_STATUS_AVG_SAMPLE(st->avg_samples) |
> @@ -159,7 +168,7 @@ static int spear_adc_read_raw(struct iio_dev *indio_dev,
>  		wait_for_completion(&st->completion); /* set by ISR */
>  		*val = st->value;
>  
> -		mutex_unlock(&indio_dev->mlock);
> +		mutex_unlock(&st->lock);
>  
>  		return IIO_VAL_INT;
>  
> @@ -187,7 +196,7 @@ static int spear_adc_write_raw(struct iio_dev *indio_dev,
>  	if (mask != IIO_CHAN_INFO_SAMP_FREQ)
>  		return -EINVAL;
>  
> -	mutex_lock(&indio_dev->mlock);
> +	mutex_lock(&st->lock);
>  
>  	if ((val < SPEAR_ADC_CLK_MIN) ||
>  	    (val > SPEAR_ADC_CLK_MAX) ||
> @@ -199,7 +208,7 @@ static int spear_adc_write_raw(struct iio_dev *indio_dev,
>  	spear_adc_set_clk(st, val);
>  
>  out:
> -	mutex_unlock(&indio_dev->mlock);
> +	mutex_unlock(&st->lock);
>  	return ret;
>  }
>  
> @@ -271,6 +280,9 @@ static int spear_adc_probe(struct platform_device *pdev)
>  	}
>  
>  	st = iio_priv(indio_dev);
> +
> +	mutex_init(&st->lock);
> +
>  	st->np = np;
>  
>  	/*