linux-kernel - Re: [PATCH] firmware: fix async/manual firmware loading

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: <CAB=NE6XBdwe7o5MKHEiNHwNVfRZ39FREwyz1gNoFXoqf=J8qYw@mail.gmail.com>
Date:   Thu, 10 Nov 2016 08:07:58 -0800
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Yves-Alexis Perez <corsac@...sac.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Yves-Alexis Perez <corsac@...ian.org>,
        Ming Lei <ming.lei@...onical.com>,
        Johannes Berg <johannes@...solutions.net>,
        Jouni Malinen <j@...fi>, Kees Cook <keescook@...omium.org>,
        Jiri Kosina <jkosina@...e.cz>, Jiri Slaby <jslaby@...e.com>,
        Tom Gundersen <teg@...m.no>, Kay Sievers <kay@...y.org>,
        Josh Boyer <jwboyer@...oraproject.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Andy Lutomirski <luto@...capital.net>,
        Harald Hoyer <harald@...hat.com>,
        Seth Forshee <seth.forshee@...onical.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Daniel Wagner <wagi@...om.org>, "4.2+" <stable@...r.kernel.org>
Subject: Re: [PATCH] firmware: fix async/manual firmware loading

On Thu, Nov 10, 2016 at 7:55 AM, Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
> On Wed, Nov 09, 2016 at 09:39:21PM +0100, Luis R. Rodriguez wrote:
>> On Sun, Oct 30, 2016 at 03:50:48PM +0100, Yves-Alexis Perez wrote:
>> > From: Yves-Alexis Perez <corsac@...ian.org>
>> >
>> > wait_for_completion_interruptible_timeout() return value is either
>> > -ERESTARTSYS (in case it was interrupted), 0 (in case the timeout expired)
>> > or the number of jiffies left until timeout. The return value is stored in
>> > a long, but in _request_firmware_load() it's silently casted to an int,
>> > which can overflow and give a negative value, indicating an error.
>> >
>> > Fix this by re-using the timeout variable and only set retval when it's
>> > safe.
>>
>> Please amend the commit log as I noted in the previous response, and
>> resend.
>>
>> > Signed-off-by: Yves-Alexis Perez <corsac@...sac.net>
>> > Cc: Ming Lei <ming.lei@...onical.com>
>> > Cc: "Luis R. Rodriguez" <mcgrof@...nel.org>
>> > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>>
>> Other than the commit log you can add on you resend:
>>
>> Acked-by: Luis R. Rodriguez.
>>
>> Modulo I don't personally thing this this is sable material but I'll let
>> Greg decide.
>
> Does it fix a regression?

Not that I am aware of, but if you consider the reported the developer then yes.

> A reported issue with an older kernel version
> that people have hit?

Definitely not.

>  It shouldn't be hard to figure out if a patch should be in stable or not...

Well with the only caveat now that I am suggesting we consider remove
this logic completely as only 2 drivers were using it explicitly
(second argument to request_firmware_nowait() set to false), it seems
they had good reasons for it but ... this has been broken for ages and
we seem to be happy to compartamentalize the UMH further, its unclear
why we would want to expand and "fix" that instead of just removing
crap that never worked. Thoughts?

  Luis