linux-kernel - Re: [PATCH v6] mfd: syscon: Add hardware spinlock support

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:   Fri, 15 Dec 2017 14:13:14 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     Baolin Wang <baolin.wang@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Mark Brown <broonie@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        DTML <devicetree@...r.kernel.org>
Subject: Re: [PATCH v6] mfd: syscon: Add hardware spinlock support

On Fri, Dec 15, 2017 at 11:42 AM, Lee Jones <lee.jones@...aro.org> wrote:

>> @@ -87,6 +88,30 @@ static struct syscon *of_syscon_register(struct device_node *np)
>>       if (ret)
>>               reg_io_width = 4;
>>
>> +     ret = of_hwspin_lock_get_id(np, 0);
>> +     if (ret > 0) {
>> +             syscon_config.hwlock_id = ret;
>> +             syscon_config.hwlock_mode = HWLOCK_IRQSTATE;
>> +     } else {
>> +             switch (ret) {
>> +             case -ENOENT:
>> +                     /* Ignore missing hwlock, it's optional. */
>> +                     break;
>> +             case 0:
>> +                     /* In case of the HWSPINLOCK is not enabled. */
>> +                     if (!IS_ENABLED(CONFIG_HWSPINLOCK))
>> +                             break;
>> +
>> +                     ret = -EINVAL;
>> +                     /* fall-through */
>> +             default:
>> +                     pr_err("Failed to retrieve valid hwlock: %d\n", ret);
>> +                     /* fall-through */
>> +             case -EPROBE_DEFER:
>> +                     goto err_regmap;
>> +             }

The 'case 0' seems odd here, are we sure that this is always a failure?
>From the of_hwspin_lock_get_id() definition it looks like zero might
be valid, and the !CONFIG_HWSPINLOCK implementation appears
to be written so that we should consider '0' valid but unused and
silently continue with that. If that is generally not the intended
use, it should probably return -EINVAL or something like that.

       Arnd