linux-kernel - Re: Clang warning in drivers/hwmon/scmi-hwmon.c

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:   Sat, 15 Sep 2018 16:37:20 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Nathan Chancellor <natechancellor@...il.com>,
        Sudeep Holla <sudeep.holla@....com>
Cc:     Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: Clang warning in drivers/hwmon/scmi-hwmon.c

On 09/14/2018 10:10 PM, Nathan Chancellor wrote:
> Hi Sundeep and others,
> 
> A group of us are trying to clean up all the warnings we see with an
> 'allyesconfig' arm64 build done with Clang and this one came up:
> 
> drivers/hwmon/scmi-hwmon.c:59:24: warning: address of array 'sensor->name' will always evaluate to 'true' [-Wpointer-bool-conversion]
>          if (sensor && sensor->name)
>                     ~~ ~~~~~~~~^~~~
> 1 warning generated.
> 
> Would it be appropriate to just delete that check or does the check
> need to be adjusted in some way (such as checking for the first value
> 'sensor->name[0]')? Either option technically solves the warning but I
> don't have the hardware to check if this breaks the driver.
> 

Presumably the check is supposed to validate the content of the data structure,
so dropping the test doesn't seem to make sense. However, one could argue
that, if the _content_ is to be checked, it should be checked earlier,
before the data pointer is stored (ie in scmi_hwmon_probe()).

Given that, I'll be happy to accept a patch to just drop the check; it
does not change the behavior of the driver, after all. If a real validation
is desirable, it can be added (into the probe function) with a separate patch.

Thanks,
Guenter