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 <zohar@linux.vnet.ibm.com> Acked-by: Kees Cook <keescook@chromium.org> Acked-by: Luis R. Rodriguez <mcgrof@kernel.org> Cc: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
parent
b844f0ecbc
commit
a1db742094
7 changed files with 22 additions and 112 deletions
|
@ -2578,6 +2578,7 @@ extern int do_pipe_flags(int *, int);
|
|||
|
||||
enum kernel_read_file_id {
|
||||
READING_FIRMWARE = 1,
|
||||
READING_MODULE,
|
||||
READING_MAX_ID
|
||||
};
|
||||
|
||||
|
|
|
@ -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;
|
||||
|
|
|
@ -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;
|
||||
|
|
|
@ -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)
|
||||
{
|
||||
|
|
|
@ -2654,7 +2654,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;
|
||||
|
||||
|
@ -2672,63 +2672,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;
|
||||
ssize_t bytes = 0;
|
||||
|
||||
if (!f.file)
|
||||
return -ENOEXEC;
|
||||
|
||||
err = security_kernel_module_from_file(f.file);
|
||||
if (err)
|
||||
goto out;
|
||||
|
||||
err = vfs_getattr(&f.file->f_path, &stat);
|
||||
if (err)
|
||||
goto out;
|
||||
|
||||
if (stat.size > INT_MAX) {
|
||||
err = -EFBIG;
|
||||
goto out;
|
||||
}
|
||||
|
||||
/* Don't hand 0 to vmalloc, it whines. */
|
||||
if (stat.size == 0) {
|
||||
err = -EINVAL;
|
||||
goto out;
|
||||
}
|
||||
|
||||
info->hdr = vmalloc(stat.size);
|
||||
if (!info->hdr) {
|
||||
err = -ENOMEM;
|
||||
goto out;
|
||||
}
|
||||
|
||||
pos = 0;
|
||||
while (pos < stat.size) {
|
||||
bytes = kernel_read(f.file, pos, (char *)(info->hdr) + pos,
|
||||
stat.size - pos);
|
||||
if (bytes < 0) {
|
||||
vfree(info->hdr);
|
||||
err = bytes;
|
||||
goto out;
|
||||
}
|
||||
if (bytes == 0)
|
||||
break;
|
||||
pos += bytes;
|
||||
}
|
||||
info->len = pos;
|
||||
|
||||
out:
|
||||
fdput(f);
|
||||
return err;
|
||||
}
|
||||
|
||||
static void free_copy(struct load_info *info)
|
||||
{
|
||||
vfree(info->hdr);
|
||||
|
@ -3589,8 +3532,10 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
|
|||
|
||||
SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
|
||||
{
|
||||
int err;
|
||||
struct load_info info = { };
|
||||
loff_t size;
|
||||
void *hdr;
|
||||
int err;
|
||||
|
||||
err = may_init_module();
|
||||
if (err)
|
||||
|
@ -3602,9 +3547,12 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
|
|||
|MODULE_INIT_IGNORE_VERMAGIC))
|
||||
return -EINVAL;
|
||||
|
||||
err = copy_module_from_fd(fd, &info);
|
||||
err = kernel_read_file_from_fd(fd, &hdr, &size, INT_MAX,
|
||||
READING_MODULE);
|
||||
if (err)
|
||||
return err;
|
||||
info.hdr = hdr;
|
||||
info.len = size;
|
||||
|
||||
return load_module(&info, uargs, flags);
|
||||
}
|
||||
|
|
|
@ -315,28 +315,6 @@ int ima_file_check(struct file *file, int mask, int opened)
|
|||
}
|
||||
EXPORT_SYMBOL_GPL(ima_file_check);
|
||||
|
||||
/**
|
||||
* ima_module_check - based on policy, collect/store/appraise measurement.
|
||||
* @file: pointer to the file to be measured/appraised
|
||||
*
|
||||
* Measure/appraise kernel modules based on policy.
|
||||
*
|
||||
* On success return 0. On integrity appraisal error, assuming the file
|
||||
* is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
|
||||
*/
|
||||
int ima_module_check(struct file *file)
|
||||
{
|
||||
if (!file) {
|
||||
#ifndef CONFIG_MODULE_SIG_FORCE
|
||||
if ((ima_appraise & IMA_APPRAISE_MODULES) &&
|
||||
(ima_appraise & IMA_APPRAISE_ENFORCE))
|
||||
return -EACCES; /* INTEGRITY_UNKNOWN */
|
||||
#endif
|
||||
return 0; /* We rely on module signature checking */
|
||||
}
|
||||
return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* ima_read_file - pre-measure/appraise hook decision based on policy
|
||||
* @file: pointer to the file to be measured/appraised/audit
|
||||
|
@ -350,6 +328,14 @@ int ima_module_check(struct file *file)
|
|||
*/
|
||||
int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
|
||||
{
|
||||
if (!file && read_id == READING_MODULE) {
|
||||
#ifndef CONFIG_MODULE_SIG_FORCE
|
||||
if ((ima_appraise & IMA_APPRAISE_MODULES) &&
|
||||
(ima_appraise & IMA_APPRAISE_ENFORCE))
|
||||
return -EACCES; /* INTEGRITY_UNKNOWN */
|
||||
#endif
|
||||
return 0; /* We rely on module signature checking */
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
@ -378,6 +364,9 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
|
|||
return 0;
|
||||
}
|
||||
|
||||
if (!file && read_id == READING_MODULE) /* MODULE_SIG_FORCE enabled */
|
||||
return 0;
|
||||
|
||||
if (!file || !buf || size == 0) { /* should never happen */
|
||||
if (ima_appraise & IMA_APPRAISE_ENFORCE)
|
||||
return -EACCES;
|
||||
|
@ -386,6 +375,8 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
|
|||
|
||||
if (read_id == READING_FIRMWARE)
|
||||
func = FIRMWARE_CHECK;
|
||||
else if (read_id == READING_MODULE)
|
||||
func = MODULE_CHECK;
|
||||
|
||||
return process_measurement(file, buf, size, MAY_READ, func, 0);
|
||||
}
|
||||
|
|
|
@ -889,16 +889,6 @@ int security_kernel_module_request(char *kmod_name)
|
|||
return call_int_hook(kernel_module_request, 0, kmod_name);
|
||||
}
|
||||
|
||||
int security_kernel_module_from_file(struct file *file)
|
||||
{
|
||||
int ret;
|
||||
|
||||
ret = call_int_hook(kernel_module_from_file, 0, file);
|
||||
if (ret)
|
||||
return ret;
|
||||
return ima_module_check(file);
|
||||
}
|
||||
|
||||
int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
|
||||
{
|
||||
int ret;
|
||||
|
@ -1705,8 +1695,6 @@ struct security_hook_heads security_hook_heads = {
|
|||
LIST_HEAD_INIT(security_hook_heads.kernel_create_files_as),
|
||||
.kernel_module_request =
|
||||
LIST_HEAD_INIT(security_hook_heads.kernel_module_request),
|
||||
.kernel_module_from_file =
|
||||
LIST_HEAD_INIT(security_hook_heads.kernel_module_from_file),
|
||||
.kernel_read_file =
|
||||
LIST_HEAD_INIT(security_hook_heads.kernel_read_file),
|
||||
.kernel_post_read_file =
|
||||
|
|
Loading…
Reference in a new issue