Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH v1 1/2] platform/x86: wmi: Break possible infinite loop
 when parsing GUID
To:     =?UTF-8?Q?Barnab=c3=a1s_P=c5=91cze?= <pobrn@protonmail.com>
Cc:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Hans de Goede <hdegoede@redhat.com>,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org,
        Mark Gross <markgross@kernel.org>
References: <20230621151155.78279-1-andriy.shevchenko@linux.intel.com>
 <25715979-8148-8d1d-fd67-a973661f9781@gmx.de>
 <TnTqU2wwXh3DG07kYUwMAe0hdBiaKiuoMOqBCBIttT27lXdw-KZVV8fZ7x-Zrg_Ux8mJUHClgyFHRbDoCRmhaOI7GwOPhUPYBRLzThV8iYI=@protonmail.com>
From:   Armin Wolf <W_Armin@gmx.de>
Message-ID: <b4dc2571-1163-805a-f92b-30dcc8b69246@gmx.de>
Date:   Wed, 21 Jun 2023 23:50:51 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <TnTqU2wwXh3DG07kYUwMAe0hdBiaKiuoMOqBCBIttT27lXdw-KZVV8fZ7x-Zrg_Ux8mJUHClgyFHRbDoCRmhaOI7GwOPhUPYBRLzThV8iYI=@protonmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Content-Language: en-US
UI-OutboundReport: notjunk:1;M01:P0:kiMdPi8m4vc=;LCPJhRcRzh4tDsBZEYMWbKcawvc
 2G7UekStaOwsb3virvIbfMVyyr4587Vjk18g53vJPa8LGW3HeiLFuRmnJGXP46I7iOk4X96As
 VLYLG8oZxNdP66mjiwtNbTP77Ishh8sJspEh9pBWDwkVh2/Y0A9FXz9DubvIdQ20inijoCvI1
 RuWB96ocDCjOHI4q6NqKw+HCWjVZaM5DWD+gjuyxA2nAaeksurii4hDLelCgCi2/xl9WJr4O2
 ZOHaGuesSYPBhkJzJ0XpMAOQRNg2/lwMMavp+QYzO52maIcbpNmaVEyvYMlSozXjqOqBgjuWf
 xICy9qJVP59HftGEpGoPLelnqHCAoXe31Z2TTzSBXYXP5y91Ai8SWCVynS7LErD18sGsLOk7T
 OlU3qT4egReVD16Szi+Sp0H28rP1P8F9HZTK8JKWFtjXjjOZi4xEEVqtUGAWdFJgZKsZDymph
 S55+tQmMQrkGUhdOczruxcOq4QZjw8CIcbPFvLxOpR+L1QYjeTJEuPRmQ4m5QUmi8rUH96vLJ
 h1ElvdVrsQWDcDWPMrXk0PfMlPnUm02v2D0D0dgQD4Y6Q1qy3Ro64dcPLEvZaQqGG7Nl/am4w
 kphyrynbEr6Eh/T8EptWhyjBbnGWwSdYt4l/UldCusNQjj/SOuTJC6i9Ftkgur2BWqSLolfUH
 p0QDRql93xSM78KsVKTIb+7yaqTblTYruDI3JfOncyLKGDbHdTB6ZxdxOzvZSDhRkq7929Ygo
 hL11twN/Ug7v4ZcZ661n5AUqzvuwSaoHTQB2MVgLxxOxxJ8t13gsrXK+MRuyGBK9harjKiI8b
 uEQVffJuaGKGNz0ETMdpIpl7PTh6MqRksBWkSCa2marHAd1Uole9/eLp+tQwDPx9nPFV3O1G9
 Lfj1rgaVj6iNDudsyIVqPJHaezrbHOpqnuJiWSdSFMDO+jA8TCkJeXoiKBELLTivizzLRa7cN
 a81KvNuOuI0eMDRAvdkuvsftlqY=
Precedence: bulk

Am 21.06.23 um 23:29 schrieb Barnab=C3=A1s P=C5=91cze:

> Hi
>
>
> 2023. j=C3=BAnius 21., szerda 23:20 keltez=C3=A9ssel, Armin Wolf <W_Armi=
n@gmx.de> =C3=ADrta:
>
>> [...]
>>> @@ -895,11 +901,7 @@ static int wmi_dev_match(struct device *dev, stru=
ct device_driver *driver)
>>>    		return 0;
>>>
>>>    	while (*id->guid_string) {
>>> -		guid_t driver_guid;
>>> -
>>> -		if (WARN_ON(guid_parse(id->guid_string, &driver_guid)))
>> Hi,
>>
>> just an idea: how about printing an error/debug message in case of an m=
alformed GUID?
>> This could be useful when searching for typos in GUIDs used by WMI driv=
ers.
>> [...]
> Wouldn't it be better to change `__wmi_driver_register()` to check that?
>
>
> Regards,
> Barnab=C3=A1s P=C5=91cze

Good point, i guess we can just forget this idea. The original motivation =
for it was the WARN_ON()
inside wmi_dev_match(), but your right that this is the wrong place to che=
ck the GUID formating.

Armin Wolf