linux-kernel - Re: [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper

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:	Thu, 2 Jun 2016 10:24:21 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Mario Limonciello <Mario_Limonciello@...l.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper

On Wed, Jun 1, 2016 at 11:35 PM,  <Mario_Limonciello@...l.com> wrote:
>> -----Original Message-----
>> From: Limonciello, Mario
>> Sent: Monday, May 16, 2016 1:28 PM
>> To: ming.lei@...onical.com
>> Cc: LKML <linux-kernel@...r.kernel.org>; Limonciello, Mario
>> <Mario_Limonciello@...l.com>
>> Subject: [PATCH v2 1/3] dell_rbu: Don't fallback to userhelper
>>
>> when loading firmware dell_rbu previously would allow a userspace
>> application to craft the payload after dell_rbu was loaded and abuse the
>> udev userspace API.
>>
>> Instead require the payload to be crafted and placed in
>> /lib/firmware/dell_rbu ahead of time.
>>
>> This adjusts dell_rbu to immediately load the firmware from
>> /lib/firmware/dell_rbu when "init" is passed into image_type using the
>> kernel helper.
>>
>> Signed-off-by: Mario Limonciello <mario_limonciello@...l.com>
>> ---
>>  drivers/firmware/Kconfig    | 1 -
>>  drivers/firmware/dell_rbu.c | 2 +-
>>  2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index
>> 6664f11..85afe59 100644
>> --- a/drivers/firmware/Kconfig
>> +++ b/drivers/firmware/Kconfig
>> @@ -86,7 +86,6 @@ config DELL_RBU
>>       tristate "BIOS update support for DELL systems via sysfs"
>>       depends on X86
>>       select FW_LOADER
>> -     select FW_LOADER_USER_HELPER
>>       help
>>        Say m if you want to have the option of updating the BIOS for your
>>        DELL system. Note you need a Dell OpenManage or Dell Update
>> package (DUP) diff --git a/drivers/firmware/dell_rbu.c
>> b/drivers/firmware/dell_rbu.c index 2f452f1..77b2a77 100644
>> --- a/drivers/firmware/dell_rbu.c
>> +++ b/drivers/firmware/dell_rbu.c
>> @@ -620,7 +620,7 @@ static ssize_t write_rbu_image_type(struct file *filp,
>> struct kobject *kobj,
>>               if (!rbu_data.entry_created) {
>>                       spin_unlock(&rbu_data.lock);
>>                       req_firm_rc =
>> request_firmware_nowait(THIS_MODULE,
>> -                             FW_ACTION_NOHOTPLUG, "dell_rbu",
>> +                             FW_ACTION_HOTPLUG, "dell_rbu",
>>                               &rbu_device->dev, GFP_KERNEL, &context,
>>                               callbackfn_rbu);
>>                       if (req_firm_rc) {
>> --
>> 2.7.4
>
> Hi Ming,
>
> Could you comment on these patches?  There isn't currently a subsystem maintainer for dell_rbu, so I think you are the most qualified to pull these in.  They have the best implications for you since you can drop all that fallback code now too.

Hi Mario,

I am fine for the 1st two patches.

But IMO it isn't good to drop support for
FW_LOADER_USER_HELPER_FALLBACK inside kernel now because udev
isn't used everywhere, and not every distribution always put the firmware files
under the default builtin path.

Thanks,