linux-kernel - Re: [PATCH 1/2] iio: dht11: Add locking

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]

Message-ID: <547DFDD8.1010404@nod.at>
Date:	Tue, 02 Dec 2014 18:58:48 +0100
From:	Richard Weinberger <richard@....at>
To:	Harald Geyer <harald@...ib.org>
CC:	jic23@...nel.org, knaack.h@....de, lars@...afoo.de,
	pmeerw@...erw.net, sanjeev_sharma@...tor.com,
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] iio: dht11: Add locking

Harald,

Am 02.12.2014 um 13:14 schrieb Harald Geyer:
>>> Move the locking out of the if statement.
>>
>> Care to explain why?
> 
> The purpose of the if statement is to limit the number of data
> transmissions if values are requested multiple times in short
> succession. (Ie an application reading both sensor values.)
> 
> If we have concurrent reads, then the later one will wait in the
> mutex_lock() while the former will update the timestamp. If the
> later one resumes, it won't check the timestamp and cause an
> unnecessary data transmission.

Okay, makes sense.
I'll update my patch!

>  
>> But I found another issue in my patch.
>> The "dht11->num_edges = -1;" before "return ret" needs to go into the locked area.
>> Will send an updated version soon.
>>
>>> BTW, it seems that there is already locking around read_raw() in the
>>> in-kernel consumer interface but not in the sysfs interface. Is there
>>> any reason for this difference?
>>
>> Dunno. :-)
> 
> If locking is actually necessary, then this would be a bug in the
> current version of the driver, which wasn't caught by at least three
> people doing reviews, so maybe let's find out if it really is necessary
> or if I'm missing something ...

Maybe IIO folks can tell us more.
What I see in other IIO drivers is that they all have locking in the read functions
and so far I see no global serialization in IIO itself.

Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/