Re: [PATCH v3 16/22] module: replace copy_module_from_fd with kernel version
On Thu, 2016-02-04 at 20:56 +0100, Luis R. Rodriguez wrote: > On Wed, Feb 03, 2016 at 02:06:24PM -0500, Mimi Zohar wrote: > > Replace copy_module_from_fd() with kernel_read_file_from_fd(). > > > > Although none of the upstreamed LSMs define a kernel_module_from_file > > hook, IMA is called, based on policy, to prevent unsigned kernel modules > > from being loaded by the original kernel module syscall and to > > measure/appraise signed kernel modules. > > > > The security function security_kernel_module_from_file() was called prior > > to reading a kernel module. Preventing unsigned kernel modules from being > > loaded by the original kernel module syscall remains on the pre-read > > kernel_read_file() security hook. Instead of reading the kernel module > > twice, once for measuring/appraising and again for loading the kernel > > module, the signature validation is moved to the kernel_post_read_file() > > security hook. > > > > This patch removes the security_kernel_module_from_file() hook and security > > call. > > > > Signed-off-by: Mimi Zohar > > Acked-by: Luis R. Rodriguez Thank you for reviewing the patches! Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 16/22] module: replace copy_module_from_fd with kernel version
On Wed, Feb 03, 2016 at 02:06:24PM -0500, Mimi Zohar wrote: > Replace copy_module_from_fd() with kernel_read_file_from_fd(). > > Although none of the upstreamed LSMs define a kernel_module_from_file > hook, IMA is called, based on policy, to prevent unsigned kernel modules > from being loaded by the original kernel module syscall and to > measure/appraise signed kernel modules. > > The security function security_kernel_module_from_file() was called prior > to reading a kernel module. Preventing unsigned kernel modules from being > loaded by the original kernel module syscall remains on the pre-read > kernel_read_file() security hook. Instead of reading the kernel module > twice, once for measuring/appraising and again for loading the kernel > module, the signature validation is moved to the kernel_post_read_file() > security hook. > > This patch removes the security_kernel_module_from_file() hook and security > call. > > Signed-off-by: Mimi Zohar Acked-by: Luis R. Rodriguez Luis ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 16/22] module: replace copy_module_from_fd with kernel version
On Wed, Feb 3, 2016 at 11:06 AM, Mimi Zohar wrote:
> Replace copy_module_from_fd() with kernel_read_file_from_fd().
>
> Although none of the upstreamed LSMs define a kernel_module_from_file
> hook, IMA is called, based on policy, to prevent unsigned kernel modules
> from being loaded by the original kernel module syscall and to
> measure/appraise signed kernel modules.
>
> The security function security_kernel_module_from_file() was called prior
> to reading a kernel module. Preventing unsigned kernel modules from being
> loaded by the original kernel module syscall remains on the pre-read
> kernel_read_file() security hook. Instead of reading the kernel module
> twice, once for measuring/appraising and again for loading the kernel
> module, the signature validation is moved to the kernel_post_read_file()
> security hook.
>
> This patch removes the security_kernel_module_from_file() hook and security
> call.
>
> Signed-off-by: Mimi Zohar
I'm really liking this consolidation. :)
Acked-by: Kees Cook
-Kees
> ---
> include/linux/fs.h| 1 +
> include/linux/ima.h | 6
> include/linux/lsm_hooks.h | 7
> include/linux/security.h | 5 ---
> kernel/module.c | 68
> +--
> security/integrity/ima/ima_main.c | 35
> security/security.c | 12 ---
> 7 files changed, 22 insertions(+), 112 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 5ba806b..9e1f1e3 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2528,6 +2528,7 @@ extern int do_pipe_flags(int *, int);
>
> enum kernel_read_file_id {
> READING_FIRMWARE = 1,
> + READING_MODULE,
> READING_MAX_ID
> };
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 6adcaea..e6516cb 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -18,7 +18,6 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
> extern int ima_file_check(struct file *file, int mask, int opened);
> extern void ima_file_free(struct file *file);
> extern int ima_file_mmap(struct file *file, unsigned long prot);
> -extern int ima_module_check(struct file *file);
> extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
> extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
> enum kernel_read_file_id id);
> @@ -44,11 +43,6 @@ static inline int ima_file_mmap(struct file *file,
> unsigned long prot)
> return 0;
> }
>
> -static inline int ima_module_check(struct file *file)
> -{
> - return 0;
> -}
> -
> static inline int ima_read_file(struct file *file, enum kernel_read_file_id
> id)
> {
> return 0;
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index d32b7bd..cdee11c 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -546,12 +546,6 @@
> * 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 kernel module from userspace.
> - * @file contains the file structure pointing to the file containing
> - * the kernel module to load. If the module is being loaded from a blob,
> - * this argument will be NULL.
> - * Return 0 if permission is granted.
> * @kernel_read_file:
> * Read a file specified by userspace.
> * @file contains the file structure pointing to the file being read
> @@ -1725,7 +1719,6 @@ struct security_hook_heads {
> struct list_head kernel_read_file;
> struct list_head kernel_post_read_file;
> struct list_head kernel_module_request;
> - struct list_head kernel_module_from_file;
> struct list_head task_fix_setuid;
> struct list_head task_setpgid;
> struct list_head task_getpgid;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 071fb74..157f0cb 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -859,11 +859,6 @@ 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_kernel_read_file(struct file *file,
> enum kernel_read_file_id id)
> {
> diff --git a/kernel/module.c b/kernel/module.c
> index 8f051a1..d77c4f1 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2665,7 +2665,7 @@ static int copy_module_from_user(const void __user
> *umod, unsigned long len,
> if (info->len < sizeof(*(info->hdr)))
> return -ENOEXEC;
>
> - err = security_kernel_module_from_file(NULL);
> + err = security_kernel_read_file(NULL, READING_MODULE);
> if (err)
>
[PATCH v3 16/22] module: replace copy_module_from_fd with kernel version
Replace copy_module_from_fd() with kernel_read_file_from_fd().
Although none of the upstreamed LSMs define a kernel_module_from_file
hook, IMA is called, based on policy, to prevent unsigned kernel modules
from being loaded by the original kernel module syscall and to
measure/appraise signed kernel modules.
The security function security_kernel_module_from_file() was called prior
to reading a kernel module. Preventing unsigned kernel modules from being
loaded by the original kernel module syscall remains on the pre-read
kernel_read_file() security hook. Instead of reading the kernel module
twice, once for measuring/appraising and again for loading the kernel
module, the signature validation is moved to the kernel_post_read_file()
security hook.
This patch removes the security_kernel_module_from_file() hook and security
call.
Signed-off-by: Mimi Zohar
---
include/linux/fs.h| 1 +
include/linux/ima.h | 6
include/linux/lsm_hooks.h | 7
include/linux/security.h | 5 ---
kernel/module.c | 68 +--
security/integrity/ima/ima_main.c | 35
security/security.c | 12 ---
7 files changed, 22 insertions(+), 112 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5ba806b..9e1f1e3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2528,6 +2528,7 @@ extern int do_pipe_flags(int *, int);
enum kernel_read_file_id {
READING_FIRMWARE = 1,
+ READING_MODULE,
READING_MAX_ID
};
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 6adcaea..e6516cb 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -18,7 +18,6 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
extern int ima_file_check(struct file *file, int mask, int opened);
extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
-extern int ima_module_check(struct file *file);
extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
enum kernel_read_file_id id);
@@ -44,11 +43,6 @@ static inline int ima_file_mmap(struct file *file, unsigned
long prot)
return 0;
}
-static inline int ima_module_check(struct file *file)
-{
- return 0;
-}
-
static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
{
return 0;
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index d32b7bd..cdee11c 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -546,12 +546,6 @@
* 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 kernel module from userspace.
- * @file contains the file structure pointing to the file containing
- * the kernel module to load. If the module is being loaded from a blob,
- * this argument will be NULL.
- * Return 0 if permission is granted.
* @kernel_read_file:
* Read a file specified by userspace.
* @file contains the file structure pointing to the file being read
@@ -1725,7 +1719,6 @@ struct security_hook_heads {
struct list_head kernel_read_file;
struct list_head kernel_post_read_file;
struct list_head kernel_module_request;
- struct list_head kernel_module_from_file;
struct list_head task_fix_setuid;
struct list_head task_setpgid;
struct list_head task_getpgid;
diff --git a/include/linux/security.h b/include/linux/security.h
index 071fb74..157f0cb 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -859,11 +859,6 @@ 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_kernel_read_file(struct file *file,
enum kernel_read_file_id id)
{
diff --git a/kernel/module.c b/kernel/module.c
index 8f051a1..d77c4f1 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2665,7 +2665,7 @@ static int copy_module_from_user(const void __user *umod,
unsigned long len,
if (info->len < sizeof(*(info->hdr)))
return -ENOEXEC;
- err = security_kernel_module_from_file(NULL);
+ err = security_kernel_read_file(NULL, READING_MODULE);
if (err)
return err;
@@ -2683,63 +2683,6 @@ static int copy_module_from_user(const void __user
*umod, unsigned long len,
return 0;
}
-/* Sets info->hdr and info->len. */
-static int copy_module_from_fd(int fd, struct load_info *info)
-{
- struct fd f = fdget(fd);
- int err;
- struct kstat stat;
- loff_t pos;
- ssiz
