linux-kernel - Re: [PATCH] input: gpio_keys: Don't report events on gpio failure

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: <CAAVeFuJ4-0O8JM2A2FWG8ad42G=LtKYet2bfESpuEPPnQikVEw@mail.gmail.com>
Date:	Mon, 31 Aug 2015 13:51:45 +0900
From:	Alexandre Courbot <gnurou@...il.com>
To:	Bjorn Andersson <bjorn.andersson@...ymobile.com>
Cc:	Linus Walleij <linus.walleij@...aro.org>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	John Stultz <john.stultz@...aro.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>
Subject: Re: [PATCH] input: gpio_keys: Don't report events on gpio failure

On Tue, Aug 18, 2015 at 4:34 AM, Bjorn Andersson
<bjorn.andersson@...ymobile.com> wrote:
> On Sun 16 Aug 23:59 PDT 2015, Alexandre Courbot wrote:
>
>> On Thu, Aug 13, 2015 at 10:06 PM, Linus Walleij
>> <linus.walleij@...aro.org> wrote:
>> > On Tue, Aug 11, 2015 at 12:41 AM, Bjorn Andersson
>> > <bjorn.andersson@...ymobile.com> wrote:
>> >
>> >> But then the question first goes to Linus & co.
>> >>
>> >> gpio_chip->get() can return a negative value to indicate errors (and did
>> >> so in this case), all parts of the API seems indicates that we can get
>> >> an error (int vs bool).
>> >
>> > Ooops.
>> >
>> >> Should we change _gpiod_get_raw_value() to propagate this error?
>> >
>> > Yes for now. Can you patch it? :)
>> >
>> >>  Or
>> >> should we just ignore this issue and propagate an error as GPIO high
>> >> reading?
>> >
>> > I don't know about the future. In some sense GPIOs are so smallish
>> > resources that errorhandling every call to read/write them seem to
>> > be a royal PITA. That is why I wanted to switch them to bool and get
>> > rid of the problem, but now I also see that maybe that was not such a
>> > smart idea, if errors do occur on the set/get_value path.
>>
>> Nowadays GPIOs may reside at the other end of an i2c bus, which means
>> that even the simplest operation like reading a GPIO value can
>> potentially fail. And it will probably not get better - wait until we
>> implement GPIO-over-IP! :)
>>
>
> Now that's progress! I can't wait ;)
>
>> So I'd say it makes sense to propagate errors returned by the driver's
>> get() hook. This might contradict some of our earlier statements about
>> simplifying the GPIO API, but is preferrable to having to make a
>> decision as to which valid value to return if the driver fails...
>>
>
> Sounds good.
>
> As we're patching up _gpiod_get_raw_value(), is the lack of a get()
> implementation the same as a LOW or is that -ENOTSUPP?

I don't see any reason why it should not be -ENOTSUPP if we start to
manage errors properly.

>
>> It should then be made very clear in the documentation that the only
>> positive values ever returned by the GPIO API will be 0 and 1 (we
>> already have a clamping mechanism for that IIRC), and that negative
>> values are propagated as-is.
>>
>
> That makes sense. I'm however not able to find such clamping
> macro/mechanism and it would be very beneficial here...
>
>> Linus, does that seem reasonable to you? Does anyone has the intention
>> to address that one or should I add it to my short-term TODO list?
>
> If you have some input on above (is lack of get() an error) I can hack
> up the patch.

Excellent - since Linus gave his thumb up, I think you can go ahead.
Looking forward to seeing this finally fixed.

Thanks!
Alex.
--
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/