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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Tue, 15 Jan 2019 09:17:58 +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

> 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.