linux-kernel - Re: [PATCH] USB: serial: cp210x: Fix GPIO in autosuspend

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <7af148c7c37e625525141bfa7d23754a@pados.hu>
Date:   Wed, 06 Feb 2019 21:43:36 +0000
From:   "Karoly Pados" <pados@...os.hu>
To:     "Johan Hovold" <johan@...nel.org>
Cc:     "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] USB: serial: cp210x: Fix GPIO in autosuspend

Uhm, yes, sorry, it kind of went out of my head. I am doing a lot of travelling lately (in the
past 48 hours I've been on 3 airplanes), and I had travels on earlier weeks too, mixed with
some project releases and family celebrations. So my head is just somewhere else.
Realistically, I'll submit something by the weekend when I'm home again.

Karoly

February 4, 2019 5:09 PM, "Johan Hovold" <johan@...nel.org> wrote:

> On Tue, Jan 15, 2019 at 11:29:42AM +0100, Johan Hovold wrote:
> 
>> On Tue, Jan 15, 2019 at 10:26:07AM +0100, Johan Hovold wrote:
>> On Tue, Jan 15, 2019 at 09:17:58AM +0000, Karoly Pados wrote:
>>>> I think it's better to add the autopm call to gpio210x_gpio_get/set
>>>> only. This will allow for a simpler patch, and keeps the autopm handling
>>>> confined to the gpio paths.
>>> 
>>> I'll submit a v2.
>>> 
>>>>> @@ -1383,6 +1397,7 @@ static void cp210x_gpio_set(struct gpio_chip *gc, unsigned int gpio, int
>>>>> value)
>>>>> } else {
>>>>> u16 wIndex = buf.state << 8 | buf.mask;
>>>>> 
>>>>> + usb_autopm_get_interface(serial->interface);
>>>> 
>>>> Also make sure to always check for errors from autopm_get().
>>> 
>>> I checked everywhere else, the reason I didn't check here is on
>>> purpose based on your previous feedback. The caller function here
>>> doesn't have a return value, so the only way to return errors is to
>>> log, but in my last patch to ftdi_sio you made clear that errors from
>>> autopm_get shouldn't get logged. Trying to call usb_control_msg() even
>>> though the device could not wake does not cause issues, and the return
>>> value from usb_control_msg() clearly identifies the reason for failure
>>> (failure due to autosuspend), so error information is not lost either.
>>> So I thought not checking here has no real disadvantage and I still
>>> stay conformant to your previous guidance.
>> 
>> Ok, I understand your reasoning, but please do check for errors and bail
>> out early if autopm_get() fails. No need to log errors.
>> 
>> Actually, we should probably add the missing error handling to the
>> callers and have gpio_set() propagate errors too. If you want to take a
>> stab at that, that could be a follow-on patch.
> 
> Karoly, did you plan on sending a v2 of this one?
> 
> Thanks,
> Johan