Re: [PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-20 Thread Kees Cook 
On Thu, Sep 20, 2012 at 1:29 PM, Andrew Morton
 wrote:
> On Fri,  7 Sep 2012 11:38:13 -0700
> Kees Cook  wrote:
>
>> 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 is a really sketchy rationale and I would like to see a *lot* more
> about the reasoning behind this.  Who will use the feature?  How will
> they use it?  What value will they obtain from using it?  This
> description should be pitched at kernel literate non-security people
> who lack mind-reading powers, please.

I'm happy to expand this further. I tried to give some examples, but
I'll expand on those. The bottom line is that Chrome OS wants to load
modules only from its crypto-verified read-only root filesystem. Doing
external per-module signatures makes no sense for us, and we want to
be able to reason about the origin of a module, which is what this
syscall gets us.

> We'll need a manpage for this, and I'd suggest that preparing it sooner
> rather than later will help with the review of your proposal.

Sounds good. The manpage is see for the old init_module looks to be
wildly out of data anyway, so perhaps I can fix that too. I'll check
the manpage tree.

>> This introduces a new syscall (currently only on x86), similar to
>> init_module, that has only two arguments. The first argument is used as
>> a file descriptor to the module and the second argument is a pointer to
>> the NULL terminated string of module arguments.
>>
>>
>> ...
>>
>> -static int copy_and_check(struct load_info *info,
>> -   const void __user *umod, unsigned long len,
>> -   const char __user *uargs)
>> +int copy_module_from_user(const void __user *umod, unsigned long len,
>> +   struct load_info *info)
>
> can be made static, methinks.

Ah, yes, thanks. I'll fix the missing statics.

> `len' should have type size_t?

Yes, this was a left-over from the prior revision of this. I'll get
that fixed too.

>> +{
>> + struct file *file;
>> + int err;
>> + struct kstat stat;
>> + unsigned long size;
>> + off_t pos;
>> + ssize_t bytes = 0;
>> +
>> + file = fget(fd);
>> + if (!file)
>> + return -ENOEXEC;
>> +
>> + err = vfs_getattr(file->f_vfsmnt, file->f_dentry, );
>> + if (err)
>> + goto out;
>> +
>> + size = stat.size;
>
> kstat.size had type loff_t.  Here it gets trucated to 32 bits on 32-bit
> machines.  Harmless I guess, but sloppy.

It's actually intentional, since I want to load the entire file into a
kmalloc-able region. I'll return EFBIG if we would truncate instead.

>> + info->hdr = vmalloc(size);
>> + if (!info->hdr) {
>> + err = -ENOMEM;
>> + goto out;
>>   }
>>
>> - if (hdr->e_shoff >= len ||
>> - hdr->e_shnum * sizeof(Elf_Shdr) > len - hdr->e_shoff) {
>> - err = -ENOEXEC;
>> - goto free_hdr;
>> + pos = 0;
>> + while (pos < size) {
>
> `pos' should be loff_t as well.
>
>> + bytes = kernel_read(file, pos, (char *)(info->hdr) + pos,
>> + size - pos);
>> + if (bytes < 0) {
>> + vfree(info->hdr);
>> + err = bytes;
>> + goto out;
>> + }
>> + if (bytes == 0)
>> + break;
>> + pos += bytes;
>>   }
>> + info->len = pos;
>> - info->hdr = hdr;
>> - info->len = len;
>> - return 0;
>> + err = check_info(info);
>> + if (err)
>> + vfree(info->hdr);
>>
>> -free_hdr:
>> - vfree(hdr);
>> +out:
>> + fput(file);
>>   return err;
>>  }
>>
>> @@ -2861,26 +2916,17 @@ static int post_relocation(struct module *mod, const 
>> struct load_info *info)
>>   return module_finalize(info->hdr, info->sechdrs, mod);
>>  }
>>
>> +static int do_init_module(struct module *mod);
>
> I wonder if do_init_module() could have been moved to avoid the forward
> declaration.

I was hoping to make the patch more readable, but I can move things
around to avoid the pre-declaration.

>
>>  /* Allocate and load the module: note that size of section 0 is always
>> zero, and we rely on this for optional sections. */
>> -static struct module *load_module(void __user *umod,
>> -   unsigned long len,
>> -   const char __user *uargs)
>> +static int load_module(struct load_info *info, const char __user *uargs)
>>  {
>> - struct load_info info = { NULL, };
>>   struct module *mod;
>>   long err;
>>
>>
>> ...
>>
>> @@ -3091,6 +3127,56 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
>>   return 0;
>>  }
>>
>> +static int

Re: [PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-20 Thread Andrew Morton 
On Fri,  7 Sep 2012 11:38:13 -0700
Kees Cook  wrote:

> 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 is a really sketchy rationale and I would like to see a *lot* more
about the reasoning behind this.  Who will use the feature?  How will
they use it?  What value will they obtain from using it?  This
description should be pitched at kernel literate non-security people
who lack mind-reading powers, please.

We'll need a manpage for this, and I'd suggest that preparing it sooner
rather than later will help with the review of your proposal.

> This introduces a new syscall (currently only on x86), similar to
> init_module, that has only two arguments. The first argument is used as
> a file descriptor to the module and the second argument is a pointer to
> the NULL terminated string of module arguments.
> 
>
> ...
>
> -static int copy_and_check(struct load_info *info,
> -   const void __user *umod, unsigned long len,
> -   const char __user *uargs)
> +int copy_module_from_user(const void __user *umod, unsigned long len,
> +   struct load_info *info)

can be made static, methinks.

`len' should have type size_t?

>  {
>   int err;
> - Elf_Ehdr *hdr;
>  
> - if (len < sizeof(*hdr))
> + info->len = len;
> + if (info->len < sizeof(*(info->hdr)))
>   return -ENOEXEC;
>  
>   /* Suck in entire file: we'll want most of it. */
> - if ((hdr = vmalloc(len)) == NULL)
> + info->hdr = vmalloc(info->len);
> + if (!info->hdr)
>   return -ENOMEM;
>  
> - if (copy_from_user(hdr, umod, len) != 0) {
> - err = -EFAULT;
> + err = copy_from_user(info->hdr, umod, info->len);
> + if (err)
>   goto free_hdr;
> - }
>  
> - /* Sanity checks against insmoding binaries or wrong arch,
> -weird elf version */
> - if (memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0
> - || hdr->e_type != ET_REL
> - || !elf_check_arch(hdr)
> - || hdr->e_shentsize != sizeof(Elf_Shdr)) {
> - err = -ENOEXEC;
> + err = check_info(info);
> + if (err)
>   goto free_hdr;
> +
> + return err;
> +
> +free_hdr:
> + vfree(info->hdr);
> + return err;
> +}
> +
> +/* Sets info->hdr and info->len. */
> +int copy_module_from_fd(int fd, struct load_info *info)

static

> +{
> + struct file *file;
> + int err;
> + struct kstat stat;
> + unsigned long size;
> + off_t pos;
> + ssize_t bytes = 0;
> +
> + file = fget(fd);
> + if (!file)
> + return -ENOEXEC;
> +
> + err = vfs_getattr(file->f_vfsmnt, file->f_dentry, );
> + if (err)
> + goto out;
> +
> + size = stat.size;

kstat.size had type loff_t.  Here it gets trucated to 32 bits on 32-bit
machines.  Harmless I guess, but sloppy.

> + info->hdr = vmalloc(size);
> + if (!info->hdr) {
> + err = -ENOMEM;
> + goto out;
>   }
>  
> - if (hdr->e_shoff >= len ||
> - hdr->e_shnum * sizeof(Elf_Shdr) > len - hdr->e_shoff) {
> - err = -ENOEXEC;
> - goto free_hdr;
> + pos = 0;
> + while (pos < size) {

`pos' should be loff_t as well.

> + bytes = kernel_read(file, pos, (char *)(info->hdr) + pos,
> + size - pos);
> + if (bytes < 0) {
> + vfree(info->hdr);
> + err = bytes;
> + goto out;
> + }
> + if (bytes == 0)
> + break;
> + pos += bytes;
>   }
> + info->len = pos;
> - info->hdr = hdr;
> - info->len = len;
> - return 0;
> + err = check_info(info);
> + if (err)
> + vfree(info->hdr);
>  
> -free_hdr:
> - vfree(hdr);
> +out:
> + fput(file);
>   return err;
>  }
>  
> @@ -2861,26 +2916,17 @@ static int post_relocation(struct module *mod, const 
> struct load_info *info)
>   return module_finalize(info->hdr, info->sechdrs, mod);
>  }
>  
> +static int do_init_module(struct module *mod);

I wonder if do_init_module() could have been moved to avoid the forward
declaration.

>  /* Allocate and load the module: note that size of section 0 is always
> zero, and we rely on this for optional sections. */
> -static struct module *load_module(void __user *umod,
> -   unsigned long len,
> -   const char __user *uargs)
> +static int load_module(struct load_info *info, const char __user *uargs)
>  {
> - struct load_info info = { NULL, };
>   struct module *mod;
>   long err;
>  
>
> ...
>
> @@ -3091,6

Re: [PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-20 Thread Andrew Morton 
On Fri,  7 Sep 2012 11:38:13 -0700
Kees Cook keesc...@chromium.org wrote:

 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.

scratches head

This is a really sketchy rationale and I would like to see a *lot* more
about the reasoning behind this.  Who will use the feature?  How will
they use it?  What value will they obtain from using it?  This
description should be pitched at kernel literate non-security people
who lack mind-reading powers, please.

We'll need a manpage for this, and I'd suggest that preparing it sooner
rather than later will help with the review of your proposal.

 This introduces a new syscall (currently only on x86), similar to
 init_module, that has only two arguments. The first argument is used as
 a file descriptor to the module and the second argument is a pointer to
 the NULL terminated string of module arguments.
 

 ...

 -static int copy_and_check(struct load_info *info,
 -   const void __user *umod, unsigned long len,
 -   const char __user *uargs)
 +int copy_module_from_user(const void __user *umod, unsigned long len,
 +   struct load_info *info)

can be made static, methinks.

`len' should have type size_t?

  {
   int err;
 - Elf_Ehdr *hdr;
  
 - if (len  sizeof(*hdr))
 + info-len = len;
 + if (info-len  sizeof(*(info-hdr)))
   return -ENOEXEC;
  
   /* Suck in entire file: we'll want most of it. */
 - if ((hdr = vmalloc(len)) == NULL)
 + info-hdr = vmalloc(info-len);
 + if (!info-hdr)
   return -ENOMEM;
  
 - if (copy_from_user(hdr, umod, len) != 0) {
 - err = -EFAULT;
 + err = copy_from_user(info-hdr, umod, info-len);
 + if (err)
   goto free_hdr;
 - }
  
 - /* Sanity checks against insmoding binaries or wrong arch,
 -weird elf version */
 - if (memcmp(hdr-e_ident, ELFMAG, SELFMAG) != 0
 - || hdr-e_type != ET_REL
 - || !elf_check_arch(hdr)
 - || hdr-e_shentsize != sizeof(Elf_Shdr)) {
 - err = -ENOEXEC;
 + err = check_info(info);
 + if (err)
   goto free_hdr;
 +
 + return err;
 +
 +free_hdr:
 + vfree(info-hdr);
 + return err;
 +}
 +
 +/* Sets info-hdr and info-len. */
 +int copy_module_from_fd(int fd, struct load_info *info)

static

 +{
 + struct file *file;
 + int err;
 + struct kstat stat;
 + unsigned long size;
 + off_t pos;
 + ssize_t bytes = 0;
 +
 + file = fget(fd);
 + if (!file)
 + return -ENOEXEC;
 +
 + err = vfs_getattr(file-f_vfsmnt, file-f_dentry, stat);
 + if (err)
 + goto out;
 +
 + size = stat.size;

kstat.size had type loff_t.  Here it gets trucated to 32 bits on 32-bit
machines.  Harmless I guess, but sloppy.

 + info-hdr = vmalloc(size);
 + if (!info-hdr) {
 + err = -ENOMEM;
 + goto out;
   }
  
 - if (hdr-e_shoff = len ||
 - hdr-e_shnum * sizeof(Elf_Shdr)  len - hdr-e_shoff) {
 - err = -ENOEXEC;
 - goto free_hdr;
 + pos = 0;
 + while (pos  size) {

`pos' should be loff_t as well.

 + bytes = kernel_read(file, pos, (char *)(info-hdr) + pos,
 + size - pos);
 + if (bytes  0) {
 + vfree(info-hdr);
 + err = bytes;
 + goto out;
 + }
 + if (bytes == 0)
 + break;
 + pos += bytes;
   }
 + info-len = pos;
 - info-hdr = hdr;
 - info-len = len;
 - return 0;
 + err = check_info(info);
 + if (err)
 + vfree(info-hdr);
  
 -free_hdr:
 - vfree(hdr);
 +out:
 + fput(file);
   return err;
  }
  
 @@ -2861,26 +2916,17 @@ static int post_relocation(struct module *mod, const 
 struct load_info *info)
   return module_finalize(info-hdr, info-sechdrs, mod);
  }
  
 +static int do_init_module(struct module *mod);

I wonder if do_init_module() could have been moved to avoid the forward
declaration.

  /* Allocate and load the module: note that size of section 0 is always
 zero, and we rely on this for optional sections. */
 -static struct module *load_module(void __user *umod,
 -   unsigned long len,
 -   const char __user *uargs)
 +static int load_module(struct load_info *info, const char __user *uargs)
  {
 - struct load_info info = { NULL, };
   struct module *mod;
   long err;
  

 ...

 @@ -3091,6 +3127,56 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
   return 0;
  }
  
 +static int init_module_permission(void)

Re: [PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-20 Thread Kees Cook 
On Thu, Sep 20, 2012 at 1:29 PM, Andrew Morton
a...@linux-foundation.org wrote:
 On Fri,  7 Sep 2012 11:38:13 -0700
 Kees Cook keesc...@chromium.org wrote:

 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.

 scratches head

 This is a really sketchy rationale and I would like to see a *lot* more
 about the reasoning behind this.  Who will use the feature?  How will
 they use it?  What value will they obtain from using it?  This
 description should be pitched at kernel literate non-security people
 who lack mind-reading powers, please.

I'm happy to expand this further. I tried to give some examples, but
I'll expand on those. The bottom line is that Chrome OS wants to load
modules only from its crypto-verified read-only root filesystem. Doing
external per-module signatures makes no sense for us, and we want to
be able to reason about the origin of a module, which is what this
syscall gets us.

 We'll need a manpage for this, and I'd suggest that preparing it sooner
 rather than later will help with the review of your proposal.

Sounds good. The manpage is see for the old init_module looks to be
wildly out of data anyway, so perhaps I can fix that too. I'll check
the manpage tree.

 This introduces a new syscall (currently only on x86), similar to
 init_module, that has only two arguments. The first argument is used as
 a file descriptor to the module and the second argument is a pointer to
 the NULL terminated string of module arguments.


 ...

 -static int copy_and_check(struct load_info *info,
 -   const void __user *umod, unsigned long len,
 -   const char __user *uargs)
 +int copy_module_from_user(const void __user *umod, unsigned long len,
 +   struct load_info *info)

 can be made static, methinks.

Ah, yes, thanks. I'll fix the missing statics.

 `len' should have type size_t?

Yes, this was a left-over from the prior revision of this. I'll get
that fixed too.

 +{
 + struct file *file;
 + int err;
 + struct kstat stat;
 + unsigned long size;
 + off_t pos;
 + ssize_t bytes = 0;
 +
 + file = fget(fd);
 + if (!file)
 + return -ENOEXEC;
 +
 + err = vfs_getattr(file-f_vfsmnt, file-f_dentry, stat);
 + if (err)
 + goto out;
 +
 + size = stat.size;

 kstat.size had type loff_t.  Here it gets trucated to 32 bits on 32-bit
 machines.  Harmless I guess, but sloppy.

It's actually intentional, since I want to load the entire file into a
kmalloc-able region. I'll return EFBIG if we would truncate instead.

 + info-hdr = vmalloc(size);
 + if (!info-hdr) {
 + err = -ENOMEM;
 + goto out;
   }

 - if (hdr-e_shoff = len ||
 - hdr-e_shnum * sizeof(Elf_Shdr)  len - hdr-e_shoff) {
 - err = -ENOEXEC;
 - goto free_hdr;
 + pos = 0;
 + while (pos  size) {

 `pos' should be loff_t as well.

 + bytes = kernel_read(file, pos, (char *)(info-hdr) + pos,
 + size - pos);
 + if (bytes  0) {
 + vfree(info-hdr);
 + err = bytes;
 + goto out;
 + }
 + if (bytes == 0)
 + break;
 + pos += bytes;
   }
 + info-len = pos;
 - info-hdr = hdr;
 - info-len = len;
 - return 0;
 + err = check_info(info);
 + if (err)
 + vfree(info-hdr);

 -free_hdr:
 - vfree(hdr);
 +out:
 + fput(file);
   return err;
  }

 @@ -2861,26 +2916,17 @@ static int post_relocation(struct module *mod, const 
 struct load_info *info)
   return module_finalize(info-hdr, info-sechdrs, mod);
  }

 +static int do_init_module(struct module *mod);

 I wonder if do_init_module() could have been moved to avoid the forward
 declaration.

I was hoping to make the patch more readable, but I can move things
around to avoid the pre-declaration.


  /* Allocate and load the module: note that size of section 0 is always
 zero, and we rely on this for optional sections. */
 -static struct module *load_module(void __user *umod,
 -   unsigned long len,
 -   const char __user *uargs)
 +static int load_module(struct load_info *info, const char __user *uargs)
  {
 - struct load_info info = { NULL, };
   struct module *mod;
   long err;


 ...

 @@ -3091,6 +3127,56 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
   return 0;
  }

 +static int init_module_permission(void)

 init_module_permission - initialises a module's permission.

 IOW, the name is poor.

 +{
 + /* Must have permission */

 The world wouldn't end if

Re: [PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-07 Thread Mimi Zohar 
On Fri, 2012-09-07 at 11:38 -0700, Kees Cook wrote:
> 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 
> Acked-by: Serge E. Hallyn 
> ---
>  include/linux/security.h |   13 +
>  kernel/module.c  |9 +
>  security/capability.c|6 ++
>  security/security.c  |5 +
>  4 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3dea6a9..368e539 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -693,6 +693,12 @@ 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 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.
>   * @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 +1513,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 +1771,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 +2285,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 c1e446e..0ad03c4 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -2430,6 +2431,10 @@ 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);
> + if (err)
> + return err;
> +

For appraisal, having the security hook here is too early.  We need to
pass both the data and the signature.

>   /* Suck in entire file: we'll want most of it. */
>   info->hdr = vmalloc(info->len);
>   if (!info->hdr)
> @@ -2468,6 +2473,10 @@ int copy_module_from_fd(int fd, struct load_info *info)
>   if (err)
>   goto out;
> 
> + err = security_kernel_module_from_file(file);
> + if (err)
> + goto out;
> +

We probably want to pass the signature here as well, for those
filesystems that don't support extended attributes.

Mimi

>   size = stat.size;
>   info->hdr = vmalloc(size);
>   if (!info->hdr) {
> 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
> ---

Re: [PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-07 Thread Eric Paris 
Acked-by: Eric Paris 

On Fri, Sep 7, 2012 at 2:38 PM, Kees Cook  wrote:
> 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 
> Acked-by: Serge E. Hallyn 
> ---
>  include/linux/security.h |   13 +
>  kernel/module.c  |9 +
>  security/capability.c|6 ++
>  security/security.c  |5 +
>  4 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3dea6a9..368e539 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -693,6 +693,12 @@ 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 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.
>   * @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 +1513,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 +1771,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 +2285,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 c1e446e..0ad03c4 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -2430,6 +2431,10 @@ 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);
> +   if (err)
> +   return err;
> +
> /* Suck in entire file: we'll want most of it. */
> info->hdr = vmalloc(info->len);
> if (!info->hdr)
> @@ -2468,6 +2473,10 @@ int copy_module_from_fd(int fd, struct load_info *info)
> if (err)
> goto out;
>
> +   err = security_kernel_module_from_file(file);
> +   if (err)
> +   goto out;
> +
> size = stat.size;
> info->hdr = vmalloc(size);
> if (!info->hdr) {
> 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)
>

[PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-07 Thread Kees Cook 
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 
Acked-by: Serge E. Hallyn 
---
 include/linux/security.h |   13 +
 kernel/module.c  |9 +
 security/capability.c|6 ++
 security/security.c  |5 +
 4 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 3dea6a9..368e539 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -693,6 +693,12 @@ 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 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.
  * @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 +1513,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 +1771,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 +2285,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 c1e446e..0ad03c4 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2430,6 +2431,10 @@ 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);
+   if (err)
+   return err;
+
/* Suck in entire file: we'll want most of it. */
info->hdr = vmalloc(info->len);
if (!info->hdr)
@@ -2468,6 +2473,10 @@ int copy_module_from_fd(int fd, struct load_info *info)
if (err)
goto out;
 
+   err = security_kernel_module_from_file(file);
+   if (err)
+   goto out;
+
size = stat.size;
info->hdr = vmalloc(size);
if (!info->hdr) {
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)
 {
--

[PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-07 Thread Kees Cook 
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 keesc...@chromium.org
Acked-by: Serge E. Hallyn serge.hal...@canonical.com
---
 include/linux/security.h |   13 +
 kernel/module.c  |9 +
 security/capability.c|6 ++
 security/security.c  |5 +
 4 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 3dea6a9..368e539 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -693,6 +693,12 @@ 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 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.
  * @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 +1513,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 +1771,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 +2285,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 c1e446e..0ad03c4 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
@@ -2430,6 +2431,10 @@ 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);
+   if (err)
+   return err;
+
/* Suck in entire file: we'll want most of it. */
info-hdr = vmalloc(info-len);
if (!info-hdr)
@@ -2468,6 +2473,10 @@ int copy_module_from_fd(int fd, struct load_info *info)
if (err)
goto out;
 
+   err = security_kernel_module_from_file(file);
+   if (err)
+   goto out;
+
size = stat.size;
info-hdr = vmalloc(size);
if (!info-hdr) {
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

Re: [PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-07 Thread Eric Paris 
Acked-by: Eric Paris epa...@redhat.com

On Fri, Sep 7, 2012 at 2:38 PM, Kees Cook keesc...@chromium.org wrote:
 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 keesc...@chromium.org
 Acked-by: Serge E. Hallyn serge.hal...@canonical.com
 ---
  include/linux/security.h |   13 +
  kernel/module.c  |9 +
  security/capability.c|6 ++
  security/security.c  |5 +
  4 files changed, 33 insertions(+), 0 deletions(-)

 diff --git a/include/linux/security.h b/include/linux/security.h
 index 3dea6a9..368e539 100644
 --- a/include/linux/security.h
 +++ b/include/linux/security.h
 @@ -693,6 +693,12 @@ 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 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.
   * @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 +1513,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 +1771,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 +2285,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 c1e446e..0ad03c4 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
 @@ -2430,6 +2431,10 @@ 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);
 +   if (err)
 +   return err;
 +
 /* Suck in entire file: we'll want most of it. */
 info-hdr = vmalloc(info-len);
 if (!info-hdr)
 @@ -2468,6 +2473,10 @@ int copy_module_from_fd(int fd, struct load_info *info)
 if (err)
 goto out;

 +   err = security_kernel_module_from_file(file);
 +   if (err)
 +   goto out;
 +
 size = stat.size;
 info-hdr = vmalloc(size);
 if (!info-hdr) {
 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 @@

Re: [PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-07 Thread Mimi Zohar 
On Fri, 2012-09-07 at 11:38 -0700, Kees Cook wrote:
 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 keesc...@chromium.org
 Acked-by: Serge E. Hallyn serge.hal...@canonical.com
 ---
  include/linux/security.h |   13 +
  kernel/module.c  |9 +
  security/capability.c|6 ++
  security/security.c  |5 +
  4 files changed, 33 insertions(+), 0 deletions(-)
 
 diff --git a/include/linux/security.h b/include/linux/security.h
 index 3dea6a9..368e539 100644
 --- a/include/linux/security.h
 +++ b/include/linux/security.h
 @@ -693,6 +693,12 @@ 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 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.
   * @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 +1513,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 +1771,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 +2285,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 c1e446e..0ad03c4 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
 @@ -2430,6 +2431,10 @@ 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);
 + if (err)
 + return err;
 +

For appraisal, having the security hook here is too early.  We need to
pass both the data and the signature.

   /* Suck in entire file: we'll want most of it. */
   info-hdr = vmalloc(info-len);
   if (!info-hdr)
 @@ -2468,6 +2473,10 @@ int copy_module_from_fd(int fd, struct load_info *info)
   if (err)
   goto out;
 
 + err = security_kernel_module_from_file(file);
 + if (err)
 + goto out;
 +

We probably want to pass the signature here as well, for those
filesystems that don't support extended attributes.

Mimi

   size = stat.size;
   info-hdr = vmalloc(size);
   if (!info-hdr) {
 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

[PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-06 Thread Kees Cook 
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 
Acked-by: Serge E. Hallyn 
---
 include/linux/security.h |   13 +
 kernel/module.c  |9 +
 security/capability.c|6 ++
 security/security.c  |5 +
 4 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 3dea6a9..368e539 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -693,6 +693,12 @@ 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 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.
  * @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 +1513,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 +1771,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 +2285,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 b080cf8..8a1aca1 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2430,6 +2431,10 @@ 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);
+   if (err)
+   return err;
+
/* Suck in entire file: we'll want most of it. */
info->hdr = vmalloc(info->len);
if (!info->hdr)
@@ -2474,6 +2479,10 @@ int copy_module_from_fd(int fd, struct load_info *info)
}
size = stat.size;
 
+   err = security_kernel_module_from_file(file);
+   if (err)
+   goto out;
+
info->hdr = vmalloc(size);
if (!info->hdr) {
err = -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)
 {
--

[PATCH 2/2] security: introduce kernel_module_from_file hook

2012-09-06 Thread Kees Cook 
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 keesc...@chromium.org
Acked-by: Serge E. Hallyn serge.hal...@canonical.com
---
 include/linux/security.h |   13 +
 kernel/module.c  |9 +
 security/capability.c|6 ++
 security/security.c  |5 +
 4 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 3dea6a9..368e539 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -693,6 +693,12 @@ 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 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.
  * @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 +1513,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 +1771,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 +2285,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 b080cf8..8a1aca1 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
@@ -2430,6 +2431,10 @@ 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);
+   if (err)
+   return err;
+
/* Suck in entire file: we'll want most of it. */
info-hdr = vmalloc(info-len);
if (!info-hdr)
@@ -2474,6 +2479,10 @@ int copy_module_from_fd(int fd, struct load_info *info)
}
size = stat.size;
 
+   err = security_kernel_module_from_file(file);
+   if (err)
+   goto out;
+
info-hdr = vmalloc(size);
if (!info-hdr) {
err = -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

Re: [PATCH 2/2] security: introduce kernel_module_from_file hook

2012-08-31 Thread Serge Hallyn 
Quoting Kees Cook (keesc...@chromium.org):
> 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 

Acked-by: Serge E. Hallyn 

> ---
>  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 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at

Re: [PATCH 2/2] security: introduce kernel_module_from_file hook

2012-08-31 Thread Serge Hallyn 
Quoting Kees Cook (keesc...@chromium.org):
 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 keesc...@chromium.org

Acked-by: Serge E. Hallyn serge.hal...@canonical.com

 ---
  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
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ

[PATCH 2/2] security: introduce kernel_module_from_file hook

2012-08-29 Thread Kees Cook 
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 
---
 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 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] security: introduce kernel_module_from_file hook

2012-08-29 Thread Kees Cook 
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 keesc...@chromium.org
---
 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

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/