Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1476162

Re: A potential race in drivers/iio/adc/vf610_adc.ko

Show all headers | View raw

On Saturday 03 September 2016 08:53 PM, Jonathan Cameron wrote:
> On 02/09/16 09:05, Pavel Andrianov wrote:
>>
> 
>> Hi!
> Hi Pavel,
>>
>> There is a potential race in drivers/iio/adc/vf610_adc.ko. Handlers
>> vf610_set_conversion_mode and vf610_write_raw are called via
>> device_attibute interface, but they are related to different
>> attributes, so may be executed in parallel. vf610_set_conversion_mode
>> acquires the mutex indio_dev->mlock, and vf610_write_raw does not.
>> Thus updating the structure 'info' may be performed simultaneously.
>>
>> Should vf610_write_raw also acquire the same mutex indio_dev->mlock?
>>
> 
> As Alison observed, mlock is not a general purpose lock for use by
> drivers. It's there to prevent state changes between direct reads
> (polled) and buffered/triggered reads (pushed).
> 
> The write raw simply sets the sampling frequency. That's not a problem
> whilst buffered capture is running or otherwise.  Interesting question
> of whether changing mode causes any trouble as well.  It's possible 
> something is undefined in the hardware during a mode change...
> 
> Anyhow, that covers mlock.  Next question: Is there a race condition in
> general?
> 
> Yes there definitely is as we have read modify write cycles
> on VF610_REG_ADC_CFG in both paths.  So what is needed is a local lock
> to protect these accesses.  Whilst in theory mlock could be used
> it should not be as it has a clearly stated purpose and using it
> for other purposes makes for much fiddlier and harder to read code!

Makes sense. What would be the best solution in this case? Should we
just introduce local lock for this module and use it for both or there
is anything we need to take care of while we have mlock for one?

> (as an aside IIRC there is no locking in sysfs attributes to prevent
> a single attribute being read twice at the same time.)
> 
> Jonathan
> 

-- 
Vaishali

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

A potential race in drivers/iio/adc/vf610_adc.ko Pavel Andrianov <andrianov@ispras.ru> - 2016-09-02 10:10 +0200
  Re: A potential race in drivers/iio/adc/vf610_adc.ko Alison Schofield <amsfield22@gmail.com> - 2016-09-02 20:10 +0200
  Re: A potential race in drivers/iio/adc/vf610_adc.ko Jonathan Cameron <jic23@kernel.org> - 2016-09-03 17:30 +0200
    Re: A potential race in drivers/iio/adc/vf610_adc.ko Vaishali Thakkar <vaishali.thakkar@oracle.com> - 2016-09-05 09:50 +0200
      Re: A potential race in drivers/iio/adc/vf610_adc.ko Jonathan Cameron <jic23@kernel.org> - 2016-09-05 22:20 +0200

csiph-web