linux-kernel - Re: [PATCH] hwmon (ina3221) Add mutex lock to shunt nodes

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] [day] [month] [year] [list]

Message-ID: <20181113044122.GA26327@Asurada-Nvidia.nvidia.com>
Date:   Mon, 12 Nov 2018 20:41:24 -0800
From:   Nicolin Chen <nicoleotsuka@...il.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     jdelvare@...e.com, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon (ina3221) Add mutex lock to shunt nodes

On Mon, Nov 12, 2018 at 08:31:22PM -0800, Guenter Roeck wrote:
> On Mon, Nov 12, 2018 at 08:23:24PM -0800, Nicolin Chen wrote:
> > The shunt resistor values are used to calculate shunt voltages
> > and currents. As a part of sysfs nodes, it would be better to
> > get protected with the same mutex too as other sysfs ABI nodes,
> > although this is not very critical because the mutex was added
> > to mainly protect register access.
> > 
> > So this patch adds the mutex lock to protect the shunt node.
> > 
> 
> I am missing something here. I don't see the point of this mutex.
> It just reads a variable and reports the result. When setting,
> it just writes a value. Protecting the conversion of the passed
> value with a mutex is pointless. Protecting writing the value
> against reading it is just as pointless.

The penalty here is that the resistor value would be mismatched
between here and the ongoing calculation when a race happens to
the sysfs accesses. But I feel it might not be really that hurt
to allow it happen.

Let's just drop it then: the mutex was used to protect register
read/writes any way.

Thanks
Nicolin