linux-kernel - Re: [PATCH v1 1/2] platform/x86: wmi: Break possible infinite loop when parsing GUID

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, 4 Jul 2023 13:02:11 +0200
From:   Hans de Goede <hdegoede@...hat.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Armin Wolf <W_Armin@....de>
Cc:     Barnabás Pőcze <pobrn@...tonmail.com>,
        platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
        Mark Gross <markgross@...nel.org>
Subject: Re: [PATCH v1 1/2] platform/x86: wmi: Break possible infinite loop
 when parsing GUID

Hi Andy,

On 6/22/23 17:00, Andy Shevchenko wrote:
> On Thu, Jun 22, 2023 at 11:43:20AM +0300, Andy Shevchenko wrote:
>> On Wed, Jun 21, 2023 at 11:50:51PM +0200, Armin Wolf wrote:
> 
> ...
> 
>> I think that WARN_ON() is a bit bogus. First of all, it can be easily
>> transformed to BUG()-equivalent with panic_on_oops and hence kill the
>> entire system. If we need the message about wrong GUID format, it should
>> be done elsewhere (modpost ?). I.o.w. we shan't expect that code,
>> controlled by us, shoots to our foot.
> 
> Additional info. There will be another driver elsewhere that may use similar
> API and also needs GUID in device ID table.
> 
> Looking into that implementation it seems that validation should be made in
> file2alias.c for WMI and reused by that driver.
> 
> So, taking into account that we have no wrong IDs so far, I would drop
> WARN_ON() here and guarantee that file2alias.c will be changed to validate
> the GUID one way or the other.
> 
> Would it work? Hans, what is your comment here?


I agree that warning on malformed GUIDs does not belong here and
your patch already drops the WARN_ON while switching to the new
guid_parse_and_compare() helper.

So I'll go and merge this into my fixes branch once rc1 is out.

Regards,

Hans