linux-kernel - Re: [PATCH v2 0/2] Add ADXL367 driver

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]

Date:   Mon, 13 Dec 2021 12:34:19 +0100
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Cosmin Tanislav <demonsingur@...il.com>
Cc:     cosmin.tanislav@...log.com,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Rob Herring <robh+dt@...nel.org>, linux-iio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 0/2] Add ADXL367 driver

On 12/7/21 10:43 AM, Cosmin Tanislav wrote:
> I have one question that is not actually specific to this driver but would
> help me clear up some issues.
>
> I used mutex_lock and mutex_unlock when accessing anything in driver's
> state that could potentially be written by another process in parallel.
>
> I heard mixed opinions about this. Some people said that it is not
> necessary to lock everywhere because loads and stores for data with size
> smaller or equal than register size would be done in one single atomic
> instruction.
>
> On the other hand, I also heard that this is not true unless WRITE_ONCE
> and READ_ONCE is used.
>
> It felt weird using WRITE_ONCE and READ_ONCE in this driver, so I kept
> using mutexes.
>
> Could I get some opinions on this matter?

What you wrote sums it up very well. READ_ONCE/WRITE_ONCE are required 
for correctness when no lock is used. The compiler is allowed to do all 
sorts of optimizations that could break multi-threading, when 
READ_ONCE/WRITE_ONCE is not used. E.g.

if (x)
   foo->bar = 10;
else
   foo->bar = 20;

Could be implemented as

foo->bar = 20;
if (x)
   foo->bar = 10;

In the absence of multi-threading the result will be the same. But if 
another thread reads foo->bar just at the right time it will read the 
incorrect 20.

For simple things like `foo->bar = x;` it is unlikely that the compiler 
will do anything other than the single store. But it could and the code 
is not correct without the WRITE_ONCE.

Using a mutex is OK, since non of this is performance critical.