Re: [PATCH RFC 0/4] Add firmware signature file check

[Takashi Iwai <tiwai@suse.de>]

At Tue, 6 Nov 2012 13:36:46 +0800,
Li Joey wrote:
> 
> [1  <text/plain; ISO-8859-1 (7bit)>]
> 2012/11/6 Ming Lei <tom.leiming@gmail.com>
> 
> > On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > >
> > > To be noted, it doesn't support the firmwares via udev but only the
> > > direct loading, and the check for built-in firmware is missing, too.
> >
> > Generally, both direct loading and udev may request one same firmware
> > image. And after check failed, current firmware load will fallback on udev
> > to complete loading, so looks a check-failed firmware still can be loaded
> > into kernel no matter if there is firmware signature check or not.
> >
> >
> > Thanks,
> > --
> > Ming Lei
> 
> 
> The udev direct write firmware through data attribute, maybe we can do the
> same signature verification in firmware_data_write? The following patch
> didn't test yet.

This would work in theory.  But in practice, when the direct file
loading fails and falls back to udev, it means that the firmware is no
file but generated somehow dynamically.  If so, a static signature
won't help, I'm afraid.


thanks,

Takashi

> 
> 
> Thanks
> Joey Lee
> 
> >From 035dde5fadc9e7f4b7811b18d3a5094ef88e8bbb Mon Sep 17 00:00:00 2001
> From: Lee, Chun-Yi <jlee@suse.com>
> Date: Tue, 6 Nov 2012 13:07:04 +0800
> Subject: [PATCH] firmware: Add signature check to firmware_data_write
> 
> ---
>  drivers/base/firmware_class.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 8945f4e..40d8cc6 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -621,6 +621,7 @@ static ssize_t firmware_data_write(struct file *filp,
> struct kobject *kobj,
>         struct firmware_priv *fw_priv = to_firmware_priv(dev);
>         struct firmware_buf *buf;
>         ssize_t retval;
> +       bool success = false;
> 
>         if (!capable(CAP_SYS_RAWIO))
>                 return -EPERM;
> @@ -655,6 +656,23 @@ static ssize_t firmware_data_write(struct file *filp,
> struct kobject *kobj,
>         }
> 
>         buf->size = max_t(size_t, offset, buf->size);
> +
> +#ifdef CONFIG_FIRMWARE_SIG
> +       for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
> +               snprintf(path, PATH_MAX, "%s/%s.sig", fw_path[i],
> buf->fw_id);
> +               if (verify_signature(buf, path))
> +                       success = true;
> +       }
> +       if (!success) {
> +               pr_err("Invalid signature file %s\n", path);
> +               if (sig_enforce) {
> +                       vfree(buf->data);
> +                       buf->data = NULL;
> +                       buf->size = 0;
> +               }
> +               retval = -ENOENT;
> +       }
> +#endif /* CONFIG_FIRMWARE_SIG */
>  out:
>         mutex_unlock(&fw_lock);
>         return retval;
> -- 
> 1.6.4.2
> [2  <text/html; ISO-8859-1 (quoted-printable)>]
>