linux-kernel - Re: [PATCH 2/4] iio: hi8435: avoid garbage event at first enable

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 23 May 2017 10:21:40 +0300
From:   Nikita Yushchenko <nikita.yoush@...entembedded.com>
To:     Vladimir Barinov <vladimir.barinov@...entembedded.com>,
        Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Matt Ranostay <mranostay@...il.com>,
        Gregor Boirie <gregor.boirie@...rot.com>,
        Sanchayan Maity <maitysanchayan@...il.com>
Cc:     linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jeff White <Jeff.White@....aero>,
        Chris Healy <Chris.Healy@....aero>
Subject: Re: [PATCH 2/4] iio: hi8435: avoid garbage event at first enable

>> +    int ret;
>> +    u32 tmp;
>> +
>> +    if (state) {
>> +        ret = hi8435_readl(priv, HI8435_SO31_0_REG, &tmp);
>> +        if (ret < 0)
>> +            return ret;
>> +        if (tmp & BIT(chan->channel))
>> +            priv->event_prev_val |= BIT(chan->channel);
>> +        else
>> +            priv->event_prev_val &= ~BIT(chan->channel);
>>   -    priv->event_scan_mask &= ~BIT(chan->channel);
>> -    if (state)
>>           priv->event_scan_mask |= BIT(chan->channel);
>> +    } else
>> +        priv->event_scan_mask &= ~BIT(chan->channel);
>>   
> 
> This will race with hi8435_iio_push_event for priv->event_scan_mask.

I was under impression that mutual-exclusion is provided by core. Now I
see it is not. Will submit a fix soon.

> Since you adding raw access then it is reasonable to initialize
> priv->event_prev_val in hi8435_read_raw.

This will cause complex interdependency between events and raw access.
E.g. should we generate event if change was discovered while processing
raw access from user space?  If we do, then we break semantics "events
generated only under trigger".  If we don't, we need complex state handling.

> Then use the following sequence for your application:
> 1. read raw value
> 2. enable events

So what if:
- one application calls read raw,
- then, far later, signal changes,
- then, far later, other application enables events?

Should second application get notification of change that happened long
before in enabled events?  I think no.

There is fundamental race between start-of-monitoring and
change-of-signal-near-start-of-monitoring. To avoid it, reading signal
must be either atomic with start of monitoring (which is not possible
with IIO API), or done after start of monitoring (which moves handling
of this race to user side).

Nikita