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] [day] [month] [year] [list]
Message-ID: <d758c48f-d77d-713b-4b10-da0bc02fc56c@redhat.com>
Date:   Mon, 6 May 2019 19:28:35 +0200
From:   Hans de Goede <hdegoede@...hat.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Darren Hart <dvhart@...radead.org>,
        Andy Shevchenko <andy@...radead.org>,
        Kai Heng Feng <kai.heng.feng@...onical.com>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] platform/x86: ideapad-laptop: Remove no_hw_rfkill_list

Hi,

On 06-05-19 14:12, Andy Shevchenko wrote:
> On Mon, Apr 29, 2019 at 5:38 PM Hans de Goede <hdegoede@...hat.com> wrote:
>>
>> When the ideapad-laptop driver was first written it was written for laptops
>> which had a hardware rfkill switch. So when the first ideapad laptops
>> showed up without a hw rfkill switch and it turned out that in this case
>> the ideapad firmware interface would always report the wifi being hardware-
>> blocked, a DMI id list of models which lack a hw rfkill switch was started
>> (by yours truly). Things were done this way to avoid regressing existing
>> models with a hw rfkill switch. In hindsight this was a mistake.
>>
>> Lenovo releases a lot of ideapad models every year and even the latest
>> models still use the "VPC2004" ACPI interface the ideapad-laptop driver
>> binds to. Having a hw rfkill switch is quite rare on modern hardware, so
>> all these new models need to be added to the no_hw_rfkill_list, leading
>> to a never ending game of whack a mole.
>>
>> Worse the failure mode when not present on the list, is very bad. In this
>> case the ideapad-laptop driver will report the wifi as being hw-blocked,
>> at which points NetworkManager does not even try to use it and the user
>> ends up with non working wifi.
>>
>> This leads to various Linux fora on the internet being filled with
>> wifi not working on ideapad laptops stories, which does not make Linux
>> look good.
>>
>> The failure mode when we flip the default to assuming that a hw rfkill
>> switch is not present OTOH is quite benign. When we properly report the
>> wifi as being hw-blocked on ideapads which do have the hw-switch; and it
>> is in the wifi-off position, then at least when using NetworkManager +
>> GNOME3 the user will get a "wifi disabled in hardware" message when trying
>> to connect to the wifi from the UI. If OTOH we assume there is no hardware
>> rfkill switch, then the user will get an empty list for the list of
>> available networks. Although the empty list vs the "wifi disabled in
>> hardware" message is a regression, it is a very minor regression and it
>> can easily be fixed on a model by model basis by filling the new
>> hw_rfkill_list this commit introduces.
>>
>> Therefor this commit removes the ever growing no_hw_rfkill_list, flipping
>> the default to assuming there is no hw rfkill switch and adding a new
>> hw_rfkill_list. Thereby fixing the wifi not working on all the current
>> ideapad and yoga models which are not on the list yet and also fixing it
>> for all future ideapad and yoga models using the "VPC2004" ACPI interface.
>>
>> Note once this patch has been accepted upstream. I plan to write a blog
>> post asking for users of ideapads and yoga's with a hw rfkill switch to
>> step forward, so that we can populate the new hw_rfkill_list with the few
>> older yoga and ideapad models which actually have a hw rfkill switch.
>>
> 
> Pushed to my review and testing queue, thanks!

Thank you, now that this is accepted / queued I will do the promised blog
post to try and get the list of devices which do actually have a hardware
rfkill switch populated.

Regards,

Hans



> 
>> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1703338
>> Signed-off-by: Hans de Goede <hdegoede@...hat.com>
>> ---
>>   drivers/platform/x86/ideapad-laptop.c | 321 ++------------------------
>>   1 file changed, 15 insertions(+), 306 deletions(-)
>>
>> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
>> index c53ae86b59c7..2d94536dea88 100644
>> --- a/drivers/platform/x86/ideapad-laptop.c
>> +++ b/drivers/platform/x86/ideapad-laptop.c
>> @@ -980,312 +980,21 @@ static void ideapad_wmi_notify(u32 value, void *context)
>>   #endif
>>
>>   /*
>> - * Some ideapads don't have a hardware rfkill switch, reading VPCCMD_R_RF
>> - * always results in 0 on these models, causing ideapad_laptop to wrongly
>> - * report all radios as hardware-blocked.
>> + * Some ideapads have a hardware rfkill switch, but most do not have one.
>> + * Reading VPCCMD_R_RF always results in 0 on models without a hardware rfkill,
>> + * switch causing ideapad_laptop to wrongly report all radios as hw-blocked.
>> + * There used to be a long list of DMI ids for models without a hw rfkill
>> + * switch here, but that resulted in playing whack a mole.
>> + * More importantly wrongly reporting the wifi radio as hw-blocked, results in
>> + * non working wifi. Whereas not reporting it hw-blocked, when it actually is
>> + * hw-blocked results in an empty SSID list, which is a much more benign
>> + * failure mode.
>> + * So the default now is the much safer option of assuming there is no
>> + * hardware rfkill switch. This default also actually matches most hardware,
>> + * since having a hw rfkill switch is quite rare on modern hardware, so this
>> + * also leads to a much shorter list.
>>    */
>> -static const struct dmi_system_id no_hw_rfkill_list[] = {
>> -       {
>> -               .ident = "Lenovo RESCUER R720-15IKBN",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo R720-15IKBN"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo G40-30",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo G40-30"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo G50-30",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo G50-30"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo V310-14IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo V310-14IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo V310-14ISK",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo V310-14ISK"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo V310-15IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo V310-15IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo V310-15ISK",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo V310-15ISK"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo V510-15IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo V510-15IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad 300-15IBR",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad 300-15IBR"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad 300-15IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad 300-15IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad 300S-11IBR",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad 300S-11BR"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad 310-15ABR",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad 310-15ABR"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad 310-15IAP",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad 310-15IAP"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad 310-15IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad 310-15IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad 310-15ISK",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad 310-15ISK"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad 330-15ICH",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad 330-15ICH"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad 530S-14ARR",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad 530S-14ARR"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad S130-14IGM",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad S130-14IGM"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad Y700-14ISK",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad Y700-14ISK"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad Y700-15ACZ",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad Y700-15ACZ"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad Y700-15ISK",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad Y700-15ISK"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad Y700 Touch-15ISK",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad Y700 Touch-15ISK"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad Y700-17ISK",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo ideapad Y700-17ISK"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo ideapad MIIX 720-12IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "MIIX 720-12IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Legion Y520-15IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Y520-15IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Y520-15IKBM",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Y520-15IKBM"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Legion Y530-15ICH",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Legion Y530-15ICH"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Legion Y530-15ICH-1060",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Legion Y530-15ICH-1060"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Legion Y720-15IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Y720-15IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Legion Y720-15IKBN",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Y720-15IKBN"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Y720-15IKBM",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Y720-15IKBM"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Yoga 2 11 / 13 / Pro",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Yoga 2"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Yoga 2 11 / 13 / Pro",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_BOARD_NAME, "Yoga2"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Yoga 2 13",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Yoga 2 13"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Yoga 3 1170 / 1470",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Yoga 3"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Yoga 3 Pro 1370",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo YOGA 3"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Yoga 700",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo YOGA 700"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Yoga 900",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo YOGA 900"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Yoga 900",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_BOARD_NAME, "VIUU4"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo YOGA 910-13IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo YOGA 910-13IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo YOGA 920-13IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo YOGA 920-13IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo YOGA C930-13IKB",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo YOGA C930-13IKB"),
>> -               },
>> -       },
>> -       {
>> -               .ident = "Lenovo Zhaoyang E42-80",
>> -               .matches = {
>> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> -                       DMI_MATCH(DMI_PRODUCT_VERSION, "ZHAOYANG E42-80"),
>> -               },
>> -       },
>> +static const struct dmi_system_id hw_rfkill_list[] = {
>>          {}
>>   };
>>
>> @@ -1311,7 +1020,7 @@ static int ideapad_acpi_add(struct platform_device *pdev)
>>          priv->cfg = cfg;
>>          priv->adev = adev;
>>          priv->platform_device = pdev;
>> -       priv->has_hw_rfkill_switch = !dmi_check_system(no_hw_rfkill_list);
>> +       priv->has_hw_rfkill_switch = dmi_check_system(hw_rfkill_list);
>>
>>          ret = ideapad_sysfs_init(priv);
>>          if (ret)
>> --
>> 2.21.0
>>
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ