| 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
| ||
|
Message-ID: <CACVXFVOnxjXjnKRWg7sv425POLSS96kAxxEohtLqj4dvcxnRyg@mail.gmail.com>
Date: Sun, 20 Jul 2014 11:04:59 +0800
From: Ming Lei <ming.lei@...onical.com>
To: Kees Cook <keescook@...omium.org>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"Luis R. Rodriguez" <mcgrof@...e.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
James Morris <james.l.morris@...cle.com>,
David Howells <dhowells@...hat.com>, linux-doc@...r.kernel.org,
linux-security-module@...r.kernel.org, linux-firmware@...nel.org,
linux-wireless <linux-wireless@...r.kernel.org>
Subject: Re: [PATCH 6/7] firmware_class: add "fd" input file
On Tue, Jul 15, 2014 at 5:38 AM, Kees Cook <keescook@...omium.org> wrote:
> As an alternative to loading bytes from the "data" blob when reading
> firmware, let kernel read from an fd, so that the LSM can reason about
> the origin of firmware contents during userspace on-demand loading.
>From user space view, maybe it is better to keep previous usage and just
check if loading is from 'data' blob or fd in 'echo 0 > loading' of
firmware_loading_store(), then the 'fd' usage becomes very similar with
before.
>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
> Documentation/firmware_class/README | 10 +++++
> drivers/base/firmware_class.c | 77 ++++++++++++++++++++++++++++++++++-
> 2 files changed, 86 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/firmware_class/README b/Documentation/firmware_class/README
> index 71f86859d7d8..d899bdacba1e 100644
> --- a/Documentation/firmware_class/README
> +++ b/Documentation/firmware_class/README
> @@ -59,6 +59,16 @@
> 8), kernel(driver): Driver code calls release_firmware(fw_entry) releasing
> the firmware image and any related resource.
>
> + Alternative loading (via file descriptor):
> + ==========================================
> + Instead of "loading", and "data" above, firmware can be loaded through
> + an open file descriptor as well, via the "fd" file:
> +
> + Replacing steps 2 thorugh 6 above with the following userspace actions,
> + performs the same loading:
> + - open firmware image file (e.g. as fd 3).
> + - write fd (e.g. "3") to /sys/class/firmware/xxx/fd.
Then as above, here should be just to replace step 4, 5 as
- open image in user space
- echo fd > /sys/class/firmware/xxx/fd
> +
> High level behavior (driver code):
> ==================================
>
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index b38cbcd6ebb1..eac996447d28 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -839,6 +839,70 @@ static struct bin_attribute firmware_attr_data = {
> .write = firmware_data_write,
> };
>
> +/**
> + * fd_store - set value in the 'fd' control file
> + * @dev: device pointer
> + * @attr: device attribute pointer
> + * @buf: buffer to scan for loading fd value
> + * @count: number of bytes in @buf
> + *
> + * Parses an fd number from buf, and then loads firmware bytes from
> + * the current process's matching open fd.
> + *
> + **/
> +static ssize_t fd_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct firmware_priv *fw_priv = to_firmware_priv(dev);
> + struct firmware_buf *fw_buf = fw_priv->buf;
> + ssize_t retval;
> + int fd;
> + struct fd f;
> +
> + if (!capable(CAP_SYS_RAWIO))
> + return -EPERM;
> +
> + retval = kstrtoint(buf, 10, &fd);
> + if (retval)
> + return retval;
> +
> + f = fdget(fd);
> + if (!f.file)
> + return -EBADF;
> +
> + mutex_lock(&fw_lock);
> + /* If we raced another loader, we must fail. */
> + if (test_bit(FW_STATUS_DONE, &fw_buf->status)) {
> + retval = -EINVAL;
> + goto unlock;
> + }
> +
> + fw_load_start(fw_buf);
> + /* fw_read_file_contents uses vmalloc, not pages. */
> + fw_buf->is_paged_buf = false;
> + retval = fw_read_file_contents(f.file, fw_buf);
> + if (retval) {
> + /* If we called fw_load_start, we must abort on fail. */
> + fw_load_abort(fw_priv);
> + goto unlock;
> + }
> +
> + /* Success. */
> + set_bit(FW_STATUS_DONE, &fw_buf->status);
> + list_del_init(&fw_buf->pending_list);
> + complete_all(&fw_buf->completion);
> +
> +unlock:
> + mutex_unlock(&fw_lock);
> + fdput(f);
> +
> + if (retval)
> + return retval;
> + return count;
> +}
> +static DEVICE_ATTR_WO(fd);
> +
> static void firmware_class_timeout_work(struct work_struct *work)
> {
> struct firmware_priv *fw_priv = container_of(work,
> @@ -912,10 +976,19 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
> mutex_lock(&fw_lock);
> list_del_init(&buf->pending_list);
> mutex_unlock(&fw_lock);
> - dev_err(f_dev, "%s: device_create_file failed\n", __func__);
> + dev_err(f_dev, "%s: dev_attr_loading failed\n", __func__);
> goto err_del_bin_attr;
> }
>
> + retval = device_create_file(f_dev, &dev_attr_fd);
> + if (retval) {
> + mutex_lock(&fw_lock);
> + list_del_init(&buf->pending_list);
> + mutex_unlock(&fw_lock);
> + dev_err(f_dev, "%s: &dev_attr_fd failed\n", __func__);
> + goto err_del_loading;
> + }
> +
> if (opt_flags & FW_OPT_UEVENT) {
> buf->need_uevent = true;
> dev_set_uevent_suppress(f_dev, false);
> @@ -933,6 +1006,8 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
> if (!buf->data)
> retval = -ENOMEM;
>
> + device_remove_file(f_dev, &dev_attr_fd);
> +err_del_loading:
> device_remove_file(f_dev, &dev_attr_loading);
> err_del_bin_attr:
> device_remove_bin_file(f_dev, &firmware_attr_data);
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists