* [PATCH v2 0/2] security: introduce kernel_fw_from_file hook
@ 2014-07-21 19:06 Kees Cook
2014-07-21 19:06 ` [PATCH v2 1/2] " Kees Cook
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Kees Cook @ 2014-07-21 19:06 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, James Morris, Greg Kroah-Hartman, Ming Lei,
linux-security-module
This is a reduced version of the original patch. This adds only the LSM
hook to the existing firmware loading logic so that the LSM can reason
about the origin and contents of a firmware coming from userspace.
Thanks!
-Kees
v2:
- further clarify header comments (jmorris)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] security: introduce kernel_fw_from_file hook
2014-07-21 19:06 [PATCH v2 0/2] security: introduce kernel_fw_from_file hook Kees Cook
@ 2014-07-21 19:06 ` Kees Cook
2014-07-21 19:06 ` [PATCH v2 2/2] firmware_class: perform new LSM checks Kees Cook
2014-07-22 14:36 ` [PATCH v2 0/2] security: introduce kernel_fw_from_file hook Mimi Zohar
2 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2014-07-21 19:06 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, James Morris, Greg Kroah-Hartman, Ming Lei,
linux-security-module
In order to validate the contents of firmware being loaded, there must be
a hook to evaluate any loaded firmware that wasn't built into the kernel
itself. Without this, there is a risk that a root user could load malicious
firmware designed to mount an attack against kernel memory (e.g. via DMA).
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/security.h | 17 +++++++++++++++++
security/capability.c | 6 ++++++
security/security.c | 6 ++++++
3 files changed, 29 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index 9c6b9722ff48..623f90e5f38d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -702,6 +702,15 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @inode points to the inode to use as a reference.
* The current task must be the one that nominated @inode.
* Return 0 if successful.
+ * @kernel_fw_from_file:
+ * Load firmware from userspace (not called for built-in firmware).
+ * @file contains the file structure pointing to the file containing
+ * the firmware to load. This argument will be NULL if the firmware
+ * was loaded via the uevent-triggered blob-based interface exposed
+ * by CONFIG_FW_LOADER_USER_HELPER.
+ * @buf pointer to buffer containing firmware contents.
+ * @size length of the firmware contents.
+ * Return 0 if permission is granted.
* @kernel_module_request:
* Ability to trigger the kernel to automatically upcall to userspace for
* userspace to load a kernel module with the given name.
@@ -1565,6 +1574,7 @@ struct security_operations {
void (*cred_transfer)(struct cred *new, const struct cred *old);
int (*kernel_act_as)(struct cred *new, u32 secid);
int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
+ int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
int (*kernel_module_request)(char *kmod_name);
int (*kernel_module_from_file)(struct file *file);
int (*task_fix_setuid) (struct cred *new, const struct cred *old,
@@ -1837,6 +1847,7 @@ int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
void security_transfer_creds(struct cred *new, const struct cred *old);
int security_kernel_act_as(struct cred *new, u32 secid);
int security_kernel_create_files_as(struct cred *new, struct inode *inode);
+int security_kernel_fw_from_file(struct file *file, char *buf, size_t size);
int security_kernel_module_request(char *kmod_name);
int security_kernel_module_from_file(struct file *file);
int security_task_fix_setuid(struct cred *new, const struct cred *old,
@@ -2363,6 +2374,12 @@ static inline int security_kernel_create_files_as(struct cred *cred,
return 0;
}
+static inline int security_kernel_fw_from_file(struct file *file,
+ char *buf, size_t size)
+{
+ return 0;
+}
+
static inline int security_kernel_module_request(char *kmod_name)
{
return 0;
diff --git a/security/capability.c b/security/capability.c
index e76373de3129..a74fde6a7468 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -401,6 +401,11 @@ static int cap_kernel_create_files_as(struct cred *new, struct inode *inode)
return 0;
}
+static int cap_kernel_fw_from_file(struct file *file, char *buf, size_t size)
+{
+ return 0;
+}
+
static int cap_kernel_module_request(char *kmod_name)
{
return 0;
@@ -1015,6 +1020,7 @@ void __init security_fixup_ops(struct security_operations *ops)
set_to_cap_if_null(ops, cred_transfer);
set_to_cap_if_null(ops, kernel_act_as);
set_to_cap_if_null(ops, kernel_create_files_as);
+ set_to_cap_if_null(ops, kernel_fw_from_file);
set_to_cap_if_null(ops, kernel_module_request);
set_to_cap_if_null(ops, kernel_module_from_file);
set_to_cap_if_null(ops, task_fix_setuid);
diff --git a/security/security.c b/security/security.c
index 31614e9e96e5..35d37d0f0d49 100644
--- a/security/security.c
+++ b/security/security.c
@@ -845,6 +845,12 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode)
return security_ops->kernel_create_files_as(new, inode);
}
+int security_kernel_fw_from_file(struct file *file, char *buf, size_t size)
+{
+ return security_ops->kernel_fw_from_file(file, buf, size);
+}
+EXPORT_SYMBOL_GPL(security_kernel_fw_from_file);
+
int security_kernel_module_request(char *kmod_name)
{
return security_ops->kernel_module_request(kmod_name);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] firmware_class: perform new LSM checks
2014-07-21 19:06 [PATCH v2 0/2] security: introduce kernel_fw_from_file hook Kees Cook
2014-07-21 19:06 ` [PATCH v2 1/2] " Kees Cook
@ 2014-07-21 19:06 ` Kees Cook
2014-07-22 2:19 ` Ming Lei
2014-07-22 6:55 ` Takashi Iwai
2014-07-22 14:36 ` [PATCH v2 0/2] security: introduce kernel_fw_from_file hook Mimi Zohar
2 siblings, 2 replies; 8+ messages in thread
From: Kees Cook @ 2014-07-21 19:06 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, James Morris, Greg Kroah-Hartman, Ming Lei,
linux-security-module
This attaches LSM hooks to the existing firmware loading interfaces:
filesystem-found firmware and demand-loaded blobs.
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/base/firmware_class.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index d276e33880be..7399bab71ced 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -28,6 +28,7 @@
#include <linux/suspend.h>
#include <linux/syscore_ops.h>
#include <linux/reboot.h>
+#include <linux/security.h>
#include <generated/utsrelease.h>
@@ -308,12 +309,17 @@ static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
if (rc != size) {
if (rc > 0)
rc = -EIO;
- vfree(buf);
- return rc;
+ goto fail;
}
+ rc = security_kernel_fw_from_file(file, buf, size);
+ if (rc)
+ goto fail;
fw_buf->data = buf;
fw_buf->size = size;
return 0;
+fail:
+ vfree(buf);
+ return rc;
}
static int fw_get_filesystem_firmware(struct device *device,
@@ -640,6 +646,12 @@ static ssize_t firmware_loading_store(struct device *dev,
break;
case 0:
if (test_bit(FW_STATUS_LOADING, &fw_buf->status)) {
+ if (security_kernel_fw_from_file(NULL, fw_buf->data,
+ fw_buf->size)) {
+ fw_load_abort(fw_priv);
+ break;
+ }
+
set_bit(FW_STATUS_DONE, &fw_buf->status);
clear_bit(FW_STATUS_LOADING, &fw_buf->status);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] firmware_class: perform new LSM checks
2014-07-21 19:06 ` [PATCH v2 2/2] firmware_class: perform new LSM checks Kees Cook
@ 2014-07-22 2:19 ` Ming Lei
2014-07-22 6:55 ` Takashi Iwai
1 sibling, 0 replies; 8+ messages in thread
From: Ming Lei @ 2014-07-22 2:19 UTC (permalink / raw)
To: Kees Cook
Cc: Linux Kernel Mailing List, James Morris, Greg Kroah-Hartman,
linux-security-module
On Tue, Jul 22, 2014 at 3:06 AM, Kees Cook <keescook@chromium.org> wrote:
> This attaches LSM hooks to the existing firmware loading interfaces:
> filesystem-found firmware and demand-loaded blobs.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/base/firmware_class.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index d276e33880be..7399bab71ced 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -28,6 +28,7 @@
> #include <linux/suspend.h>
> #include <linux/syscore_ops.h>
> #include <linux/reboot.h>
> +#include <linux/security.h>
>
> #include <generated/utsrelease.h>
>
> @@ -308,12 +309,17 @@ static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
> if (rc != size) {
> if (rc > 0)
> rc = -EIO;
> - vfree(buf);
> - return rc;
> + goto fail;
> }
> + rc = security_kernel_fw_from_file(file, buf, size);
> + if (rc)
> + goto fail;
> fw_buf->data = buf;
> fw_buf->size = size;
> return 0;
> +fail:
> + vfree(buf);
> + return rc;
> }
>
> static int fw_get_filesystem_firmware(struct device *device,
> @@ -640,6 +646,12 @@ static ssize_t firmware_loading_store(struct device *dev,
> break;
> case 0:
> if (test_bit(FW_STATUS_LOADING, &fw_buf->status)) {
> + if (security_kernel_fw_from_file(NULL, fw_buf->data,
> + fw_buf->size)) {
> + fw_load_abort(fw_priv);
> + break;
It may be more friendly to return -EPERM or sort of failure
to user, otherwise it might cause userspace loader a bit confused.
Thanks,
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] firmware_class: perform new LSM checks
2014-07-21 19:06 ` [PATCH v2 2/2] firmware_class: perform new LSM checks Kees Cook
2014-07-22 2:19 ` Ming Lei
@ 2014-07-22 6:55 ` Takashi Iwai
2014-07-22 17:39 ` Kees Cook
1 sibling, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2014-07-22 6:55 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, James Morris, Greg Kroah-Hartman, Ming Lei,
linux-security-module
At Mon, 21 Jul 2014 12:06:41 -0700,
Kees Cook wrote:
>
> This attaches LSM hooks to the existing firmware loading interfaces:
> filesystem-found firmware and demand-loaded blobs.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/base/firmware_class.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index d276e33880be..7399bab71ced 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -28,6 +28,7 @@
> #include <linux/suspend.h>
> #include <linux/syscore_ops.h>
> #include <linux/reboot.h>
> +#include <linux/security.h>
>
> #include <generated/utsrelease.h>
>
> @@ -308,12 +309,17 @@ static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
> if (rc != size) {
> if (rc > 0)
> rc = -EIO;
> - vfree(buf);
> - return rc;
> + goto fail;
> }
> + rc = security_kernel_fw_from_file(file, buf, size);
> + if (rc)
> + goto fail;
> fw_buf->data = buf;
> fw_buf->size = size;
> return 0;
> +fail:
> + vfree(buf);
> + return rc;
> }
>
> static int fw_get_filesystem_firmware(struct device *device,
> @@ -640,6 +646,12 @@ static ssize_t firmware_loading_store(struct device *dev,
> break;
> case 0:
> if (test_bit(FW_STATUS_LOADING, &fw_buf->status)) {
> + if (security_kernel_fw_from_file(NULL, fw_buf->data,
> + fw_buf->size)) {
> + fw_load_abort(fw_priv);
> + break;
> + }
> set_bit(FW_STATUS_DONE, &fw_buf->status);
> clear_bit(FW_STATUS_LOADING, &fw_buf->status);
security_kernel_fw_from_file() should be called after
fw_map_pages_buf() call (that is found after these lines).
Otherwise fw_buf->buf won't contain a valid buffer pointer.
Takashi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/2] security: introduce kernel_fw_from_file hook
2014-07-21 19:06 [PATCH v2 0/2] security: introduce kernel_fw_from_file hook Kees Cook
2014-07-21 19:06 ` [PATCH v2 1/2] " Kees Cook
2014-07-21 19:06 ` [PATCH v2 2/2] firmware_class: perform new LSM checks Kees Cook
@ 2014-07-22 14:36 ` Mimi Zohar
2 siblings, 0 replies; 8+ messages in thread
From: Mimi Zohar @ 2014-07-22 14:36 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, James Morris, Greg Kroah-Hartman, Ming Lei,
linux-security-module, Vivek Goyal
On Mon, 2014-07-21 at 12:06 -0700, Kees Cook wrote:
> This is a reduced version of the original patch. This adds only the LSM
> hook to the existing firmware loading logic so that the LSM can reason
> about the origin and contents of a firmware coming from userspace.
>
> Thanks!
>
> -Kees
>
> v2:
> - further clarify header comments (jmorris)
Thanks Kees! We now have module and firmware security hooks. What's
next - kexec?
Please feel free to add my Acks.
Mimi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] firmware_class: perform new LSM checks
2014-07-22 6:55 ` Takashi Iwai
@ 2014-07-22 17:39 ` Kees Cook
2014-07-23 10:11 ` Takashi Iwai
0 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2014-07-22 17:39 UTC (permalink / raw)
To: Takashi Iwai
Cc: LKML, James Morris, Greg Kroah-Hartman, Ming Lei, linux-security-module
On Mon, Jul 21, 2014 at 11:55 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Mon, 21 Jul 2014 12:06:41 -0700,
> Kees Cook wrote:
>>
>> This attaches LSM hooks to the existing firmware loading interfaces:
>> filesystem-found firmware and demand-loaded blobs.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> ---
>> drivers/base/firmware_class.c | 16 ++++++++++++++--
>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
>> index d276e33880be..7399bab71ced 100644
>> --- a/drivers/base/firmware_class.c
>> +++ b/drivers/base/firmware_class.c
>> @@ -28,6 +28,7 @@
>> #include <linux/suspend.h>
>> #include <linux/syscore_ops.h>
>> #include <linux/reboot.h>
>> +#include <linux/security.h>
>>
>> #include <generated/utsrelease.h>
>>
>> @@ -308,12 +309,17 @@ static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
>> if (rc != size) {
>> if (rc > 0)
>> rc = -EIO;
>> - vfree(buf);
>> - return rc;
>> + goto fail;
>> }
>> + rc = security_kernel_fw_from_file(file, buf, size);
>> + if (rc)
>> + goto fail;
>> fw_buf->data = buf;
>> fw_buf->size = size;
>> return 0;
>> +fail:
>> + vfree(buf);
>> + return rc;
>> }
>>
>> static int fw_get_filesystem_firmware(struct device *device,
>> @@ -640,6 +646,12 @@ static ssize_t firmware_loading_store(struct device *dev,
>> break;
>> case 0:
>> if (test_bit(FW_STATUS_LOADING, &fw_buf->status)) {
>> + if (security_kernel_fw_from_file(NULL, fw_buf->data,
>> + fw_buf->size)) {
>> + fw_load_abort(fw_priv);
>> + break;
>> + }
>> set_bit(FW_STATUS_DONE, &fw_buf->status);
>> clear_bit(FW_STATUS_LOADING, &fw_buf->status);
>
> security_kernel_fw_from_file() should be called after
> fw_map_pages_buf() call (that is found after these lines).
> Otherwise fw_buf->buf won't contain a valid buffer pointer.
Ah! Good to know. I guess I was getting lucky in my testing. Is this a
race condition?
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] firmware_class: perform new LSM checks
2014-07-22 17:39 ` Kees Cook
@ 2014-07-23 10:11 ` Takashi Iwai
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2014-07-23 10:11 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, James Morris, Greg Kroah-Hartman, Ming Lei, linux-security-module
At Tue, 22 Jul 2014 10:39:00 -0700,
Kees Cook wrote:
>
> On Mon, Jul 21, 2014 at 11:55 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > At Mon, 21 Jul 2014 12:06:41 -0700,
> > Kees Cook wrote:
> >>
> >> This attaches LSM hooks to the existing firmware loading interfaces:
> >> filesystem-found firmware and demand-loaded blobs.
> >>
> >> Signed-off-by: Kees Cook <keescook@chromium.org>
> >> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> ---
> >> drivers/base/firmware_class.c | 16 ++++++++++++++--
> >> 1 file changed, 14 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> >> index d276e33880be..7399bab71ced 100644
> >> --- a/drivers/base/firmware_class.c
> >> +++ b/drivers/base/firmware_class.c
> >> @@ -28,6 +28,7 @@
> >> #include <linux/suspend.h>
> >> #include <linux/syscore_ops.h>
> >> #include <linux/reboot.h>
> >> +#include <linux/security.h>
> >>
> >> #include <generated/utsrelease.h>
> >>
> >> @@ -308,12 +309,17 @@ static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
> >> if (rc != size) {
> >> if (rc > 0)
> >> rc = -EIO;
> >> - vfree(buf);
> >> - return rc;
> >> + goto fail;
> >> }
> >> + rc = security_kernel_fw_from_file(file, buf, size);
> >> + if (rc)
> >> + goto fail;
> >> fw_buf->data = buf;
> >> fw_buf->size = size;
> >> return 0;
> >> +fail:
> >> + vfree(buf);
> >> + return rc;
> >> }
> >>
> >> static int fw_get_filesystem_firmware(struct device *device,
> >> @@ -640,6 +646,12 @@ static ssize_t firmware_loading_store(struct device *dev,
> >> break;
> >> case 0:
> >> if (test_bit(FW_STATUS_LOADING, &fw_buf->status)) {
> >> + if (security_kernel_fw_from_file(NULL, fw_buf->data,
> >> + fw_buf->size)) {
> >> + fw_load_abort(fw_priv);
> >> + break;
> >> + }
> >> set_bit(FW_STATUS_DONE, &fw_buf->status);
> >> clear_bit(FW_STATUS_LOADING, &fw_buf->status);
> >
> > security_kernel_fw_from_file() should be called after
> > fw_map_pages_buf() call (that is found after these lines).
> > Otherwise fw_buf->buf won't contain a valid buffer pointer.
>
> Ah! Good to know. I guess I was getting lucky in my testing. Is this a
> race condition?
This is the code path where direct f/w loading fails but the
user-space helper feeds the data. Did you test that scenario?
Takashi
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-07-23 10:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-21 19:06 [PATCH v2 0/2] security: introduce kernel_fw_from_file hook Kees Cook
2014-07-21 19:06 ` [PATCH v2 1/2] " Kees Cook
2014-07-21 19:06 ` [PATCH v2 2/2] firmware_class: perform new LSM checks Kees Cook
2014-07-22 2:19 ` Ming Lei
2014-07-22 6:55 ` Takashi Iwai
2014-07-22 17:39 ` Kees Cook
2014-07-23 10:11 ` Takashi Iwai
2014-07-22 14:36 ` [PATCH v2 0/2] security: introduce kernel_fw_from_file hook Mimi Zohar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.