* [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 [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 [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 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
* 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®