Instead of (or in addition to) kernel module signing, being able to reason
about the origin of a kernel module would be valuable in situations
where an OS already trusts a specific file system, file, etc, due to
things like security labels or an existing root of trust to a partition
through things like dm-verity.
This changes the init_module syscall so that when the first argument
(blob address) is NULL, the second argument is used as a file descriptor
to the module (instead of length). The third argument (module arguments)
remains unchanged.
Some alternatives to overloading the existing syscall are:
- write a new syscall (seemed unnecessary)
- add an fd ioctl (awful)
- enhance the ELF binfmt loader (complex)
It seemed most sensible to avoid introducing new or crazy interfaces
or further complicating the ELF loader. Instead, just use the existing
syscall in a new way. Tools using the fd argument style can trivially
downgrade to the blob argument style when they see an EFAULT error.
Signed-off-by: Kees Cook <[email protected]>
---
kernel/module.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 87 insertions(+), 10 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 4edbd9c..0be8c11 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -21,6 +21,7 @@
#include <linux/ftrace_event.h>
#include <linux/init.h>
#include <linux/kallsyms.h>
+#include <linux/file.h>
#include <linux/fs.h>
#include <linux/sysfs.h>
#include <linux/kernel.h>
@@ -2399,23 +2400,99 @@ static inline void kmemleak_load_module(const struct module *mod,
}
#endif
-/* Sets info->hdr and info->len. */
-static int copy_and_check(struct load_info *info,
- const void __user *umod, unsigned long len,
- const char __user *uargs)
+static Elf_Ehdr *copy_module_from_user(const void __user *umod,
+ unsigned long len)
{
- int err;
Elf_Ehdr *hdr;
if (len < sizeof(*hdr))
- return -ENOEXEC;
+ return ERR_PTR(-ENOEXEC);
/* Suck in entire file: we'll want most of it. */
- if ((hdr = vmalloc(len)) == NULL)
- return -ENOMEM;
+ hdr = vmalloc(len);
+ if (!hdr)
+ return ERR_PTR(-ENOMEM);
if (copy_from_user(hdr, umod, len) != 0) {
- err = -EFAULT;
+ vfree(hdr);
+ return ERR_PTR(-EFAULT);
+ }
+
+ return hdr;
+}
+
+static Elf_Ehdr *copy_module_from_fd(unsigned int fd, unsigned long *len)
+{
+ struct file *file;
+ int err;
+ Elf_Ehdr *hdr;
+ struct kstat stat;
+ unsigned long size;
+ off_t pos;
+ ssize_t bytes = 0;
+
+ file = fget(fd);
+ if (!file)
+ return ERR_PTR(-ENOEXEC);
+
+ err = vfs_getattr(file->f_vfsmnt, file->f_dentry, &stat);
+ if (err) {
+ hdr = ERR_PTR(err);
+ goto out;
+ }
+
+ if (stat.size > INT_MAX) {
+ hdr = ERR_PTR(-ENOMEM);
+ goto out;
+ }
+ size = stat.size;
+
+ hdr = vmalloc(size);
+ if (!hdr) {
+ hdr = ERR_PTR(-ENOMEM);
+ goto out;
+ }
+
+ pos = 0;
+ while (pos < size) {
+ bytes = kernel_read(file, pos, (char *)hdr + pos, size - pos);
+ if (bytes < 0) {
+ vfree(hdr);
+ hdr = ERR_PTR(bytes);
+ goto out;
+ }
+ if (bytes == 0)
+ break;
+ pos += bytes;
+ }
+ *len = pos;
+
+out:
+ fput(file);
+ return hdr;
+}
+
+/* Sets info->hdr and info->len. */
+static int copy_and_check(struct load_info *info,
+ const void __user *umod, unsigned long len)
+{
+ int err;
+ Elf_Ehdr *hdr;
+
+ if (umod == NULL) {
+ unsigned int fd;
+
+ if (len < 0 || len > INT_MAX)
+ return -ENOEXEC;
+ fd = len;
+
+ hdr = copy_module_from_fd(fd, &len);
+ } else
+ hdr = copy_module_from_user(umod, len);
+ if (IS_ERR(hdr))
+ return PTR_ERR(hdr);
+ if (len < sizeof(*hdr)) {
+ err = -ENOEXEC;
goto free_hdr;
}
@@ -2875,7 +2952,7 @@ static struct module *load_module(void __user *umod,
umod, len, uargs);
/* Copy in the blobs from userspace, check they are vaguely sane. */
- err = copy_and_check(&info, umod, len, uargs);
+ err = copy_and_check(&info, umod, len);
if (err)
return ERR_PTR(err);
--
1.7.0.4
Now that kernel module origins can be reasoned about, provide a hook to
the LSMs to make policy decisions about the module file.
Signed-off-by: Kees Cook <[email protected]>
---
include/linux/security.h | 11 +++++++++++
kernel/module.c | 7 +++++++
security/capability.c | 6 ++++++
security/security.c | 5 +++++
4 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 3dea6a9..634d09a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -693,6 +693,10 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* userspace to load a kernel module with the given name.
* @kmod_name name of the module requested by the kernel
* Return 0 if successful.
+ * @kernel_module_from_file:
+ * Load a new kernel module from a file.
+ * @file contains the file structure being loaded as a kernel module.
+ * Return 0 if permission is granted.
* @task_fix_setuid:
* Update the module's state after setting one or more of the user
* identity attributes of the current process. The @flags parameter
@@ -1507,6 +1511,7 @@ struct security_operations {
int (*kernel_act_as)(struct cred *new, u32 secid);
int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
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,
int flags);
int (*task_setpgid) (struct task_struct *p, pid_t pgid);
@@ -1764,6 +1769,7 @@ 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_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,
int flags);
int security_task_setpgid(struct task_struct *p, pid_t pgid);
@@ -2277,6 +2283,11 @@ static inline int security_kernel_module_request(char *kmod_name)
return 0;
}
+static inline int security_kernel_module_from_file(struct file *file)
+{
+ return 0;
+}
+
static inline int security_task_fix_setuid(struct cred *new,
const struct cred *old,
int flags)
diff --git a/kernel/module.c b/kernel/module.c
index 0be8c11..1fcc63f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -29,6 +29,7 @@
#include <linux/vmalloc.h>
#include <linux/elf.h>
#include <linux/proc_fs.h>
+#include <linux/security.h>
#include <linux/seq_file.h>
#include <linux/syscalls.h>
#include <linux/fcntl.h>
@@ -2447,6 +2448,12 @@ static Elf_Ehdr *copy_module_from_fd(unsigned int fd, unsigned long *len)
}
size = stat.size;
+ err = security_kernel_module_from_file(file);
+ if (err) {
+ hdr = ERR_PTR(err);
+ goto out;
+ }
+
hdr = vmalloc(size);
if (!hdr) {
hdr = ERR_PTR(-ENOMEM);
diff --git a/security/capability.c b/security/capability.c
index 61095df..8acb304 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -395,6 +395,11 @@ static int cap_kernel_module_request(char *kmod_name)
return 0;
}
+static int cap_kernel_module_from_file(struct file *file)
+{
+ return 0;
+}
+
static int cap_task_setpgid(struct task_struct *p, pid_t pgid)
{
return 0;
@@ -967,6 +972,7 @@ void __init security_fixup_ops(struct security_operations *ops)
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_module_request);
+ set_to_cap_if_null(ops, kernel_module_from_file);
set_to_cap_if_null(ops, task_fix_setuid);
set_to_cap_if_null(ops, task_setpgid);
set_to_cap_if_null(ops, task_getpgid);
diff --git a/security/security.c b/security/security.c
index 860aeb3..f7f8695 100644
--- a/security/security.c
+++ b/security/security.c
@@ -799,6 +799,11 @@ int security_kernel_module_request(char *kmod_name)
return security_ops->kernel_module_request(kmod_name);
}
+int security_kernel_module_from_file(struct file *file)
+{
+ return security_ops->kernel_module_from_file(file);
+}
+
int security_task_fix_setuid(struct cred *new, const struct cred *old,
int flags)
{
--
1.7.0.4
Quoting Kees Cook ([email protected]):
> Instead of (or in addition to) kernel module signing, being able to reason
> about the origin of a kernel module would be valuable in situations
> where an OS already trusts a specific file system, file, etc, due to
> things like security labels or an existing root of trust to a partition
> through things like dm-verity.
>
> This changes the init_module syscall so that when the first argument
> (blob address) is NULL, the second argument is used as a file descriptor
> to the module (instead of length). The third argument (module arguments)
> remains unchanged.
>
> Some alternatives to overloading the existing syscall are:
> - write a new syscall (seemed unnecessary)
> - add an fd ioctl (awful)
> - enhance the ELF binfmt loader (complex)
>
> It seemed most sensible to avoid introducing new or crazy interfaces
> or further complicating the ELF loader. Instead, just use the existing
> syscall in a new way. Tools using the fd argument style can trivially
> downgrade to the blob argument style when they see an EFAULT error.
>
> Signed-off-by: Kees Cook <[email protected]>
Acked-by: Serge E. Hallyn <[email protected]>
> ---
> kernel/module.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 files changed, 87 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 4edbd9c..0be8c11 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -21,6 +21,7 @@
> #include <linux/ftrace_event.h>
> #include <linux/init.h>
> #include <linux/kallsyms.h>
> +#include <linux/file.h>
> #include <linux/fs.h>
> #include <linux/sysfs.h>
> #include <linux/kernel.h>
> @@ -2399,23 +2400,99 @@ static inline void kmemleak_load_module(const struct module *mod,
> }
> #endif
>
> -/* Sets info->hdr and info->len. */
> -static int copy_and_check(struct load_info *info,
> - const void __user *umod, unsigned long len,
> - const char __user *uargs)
> +static Elf_Ehdr *copy_module_from_user(const void __user *umod,
> + unsigned long len)
> {
> - int err;
> Elf_Ehdr *hdr;
>
> if (len < sizeof(*hdr))
> - return -ENOEXEC;
> + return ERR_PTR(-ENOEXEC);
>
> /* Suck in entire file: we'll want most of it. */
> - if ((hdr = vmalloc(len)) == NULL)
> - return -ENOMEM;
> + hdr = vmalloc(len);
> + if (!hdr)
> + return ERR_PTR(-ENOMEM);
>
> if (copy_from_user(hdr, umod, len) != 0) {
> - err = -EFAULT;
> + vfree(hdr);
> + return ERR_PTR(-EFAULT);
> + }
> +
> + return hdr;
> +}
> +
> +static Elf_Ehdr *copy_module_from_fd(unsigned int fd, unsigned long *len)
> +{
> + struct file *file;
> + int err;
> + Elf_Ehdr *hdr;
> + struct kstat stat;
> + unsigned long size;
> + off_t pos;
> + ssize_t bytes = 0;
> +
> + file = fget(fd);
> + if (!file)
> + return ERR_PTR(-ENOEXEC);
> +
> + err = vfs_getattr(file->f_vfsmnt, file->f_dentry, &stat);
> + if (err) {
> + hdr = ERR_PTR(err);
> + goto out;
> + }
> +
> + if (stat.size > INT_MAX) {
> + hdr = ERR_PTR(-ENOMEM);
> + goto out;
> + }
> + size = stat.size;
> +
> + hdr = vmalloc(size);
> + if (!hdr) {
> + hdr = ERR_PTR(-ENOMEM);
> + goto out;
> + }
> +
> + pos = 0;
> + while (pos < size) {
> + bytes = kernel_read(file, pos, (char *)hdr + pos, size - pos);
> + if (bytes < 0) {
> + vfree(hdr);
> + hdr = ERR_PTR(bytes);
> + goto out;
> + }
> + if (bytes == 0)
> + break;
> + pos += bytes;
> + }
> + *len = pos;
> +
> +out:
> + fput(file);
> + return hdr;
> +}
> +
> +/* Sets info->hdr and info->len. */
> +static int copy_and_check(struct load_info *info,
> + const void __user *umod, unsigned long len)
> +{
> + int err;
> + Elf_Ehdr *hdr;
> +
> + if (umod == NULL) {
> + unsigned int fd;
> +
> + if (len < 0 || len > INT_MAX)
> + return -ENOEXEC;
> + fd = len;
> +
> + hdr = copy_module_from_fd(fd, &len);
> + } else
> + hdr = copy_module_from_user(umod, len);
> + if (IS_ERR(hdr))
> + return PTR_ERR(hdr);
> + if (len < sizeof(*hdr)) {
> + err = -ENOEXEC;
> goto free_hdr;
> }
>
> @@ -2875,7 +2952,7 @@ static struct module *load_module(void __user *umod,
> umod, len, uargs);
>
> /* Copy in the blobs from userspace, check they are vaguely sane. */
> - err = copy_and_check(&info, umod, len, uargs);
> + err = copy_and_check(&info, umod, len);
> if (err)
> return ERR_PTR(err);
>
> --
> 1.7.0.4
>
Quoting Kees Cook ([email protected]):
> Now that kernel module origins can be reasoned about, provide a hook to
> the LSMs to make policy decisions about the module file.
>
> Signed-off-by: Kees Cook <[email protected]>
Acked-by: Serge E. Hallyn <[email protected]>
> ---
> include/linux/security.h | 11 +++++++++++
> kernel/module.c | 7 +++++++
> security/capability.c | 6 ++++++
> security/security.c | 5 +++++
> 4 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3dea6a9..634d09a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -693,6 +693,10 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
> * userspace to load a kernel module with the given name.
> * @kmod_name name of the module requested by the kernel
> * Return 0 if successful.
> + * @kernel_module_from_file:
> + * Load a new kernel module from a file.
> + * @file contains the file structure being loaded as a kernel module.
> + * Return 0 if permission is granted.
> * @task_fix_setuid:
> * Update the module's state after setting one or more of the user
> * identity attributes of the current process. The @flags parameter
> @@ -1507,6 +1511,7 @@ struct security_operations {
> int (*kernel_act_as)(struct cred *new, u32 secid);
> int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
> 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,
> int flags);
> int (*task_setpgid) (struct task_struct *p, pid_t pgid);
> @@ -1764,6 +1769,7 @@ 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_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,
> int flags);
> int security_task_setpgid(struct task_struct *p, pid_t pgid);
> @@ -2277,6 +2283,11 @@ static inline int security_kernel_module_request(char *kmod_name)
> return 0;
> }
>
> +static inline int security_kernel_module_from_file(struct file *file)
> +{
> + return 0;
> +}
> +
> static inline int security_task_fix_setuid(struct cred *new,
> const struct cred *old,
> int flags)
> diff --git a/kernel/module.c b/kernel/module.c
> index 0be8c11..1fcc63f 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -29,6 +29,7 @@
> #include <linux/vmalloc.h>
> #include <linux/elf.h>
> #include <linux/proc_fs.h>
> +#include <linux/security.h>
> #include <linux/seq_file.h>
> #include <linux/syscalls.h>
> #include <linux/fcntl.h>
> @@ -2447,6 +2448,12 @@ static Elf_Ehdr *copy_module_from_fd(unsigned int fd, unsigned long *len)
> }
> size = stat.size;
>
> + err = security_kernel_module_from_file(file);
> + if (err) {
> + hdr = ERR_PTR(err);
> + goto out;
> + }
> +
> hdr = vmalloc(size);
> if (!hdr) {
> hdr = ERR_PTR(-ENOMEM);
> diff --git a/security/capability.c b/security/capability.c
> index 61095df..8acb304 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -395,6 +395,11 @@ static int cap_kernel_module_request(char *kmod_name)
> return 0;
> }
>
> +static int cap_kernel_module_from_file(struct file *file)
> +{
> + return 0;
> +}
> +
> static int cap_task_setpgid(struct task_struct *p, pid_t pgid)
> {
> return 0;
> @@ -967,6 +972,7 @@ void __init security_fixup_ops(struct security_operations *ops)
> 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_module_request);
> + set_to_cap_if_null(ops, kernel_module_from_file);
> set_to_cap_if_null(ops, task_fix_setuid);
> set_to_cap_if_null(ops, task_setpgid);
> set_to_cap_if_null(ops, task_getpgid);
> diff --git a/security/security.c b/security/security.c
> index 860aeb3..f7f8695 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -799,6 +799,11 @@ int security_kernel_module_request(char *kmod_name)
> return security_ops->kernel_module_request(kmod_name);
> }
>
> +int security_kernel_module_from_file(struct file *file)
> +{
> + return security_ops->kernel_module_from_file(file);
> +}
> +
> int security_task_fix_setuid(struct cred *new, const struct cred *old,
> int flags)
> {
> --
> 1.7.0.4
>