Kees Cook <keesc...@chromium.org> writes:
> 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 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.
Thanks. Minor comments follow:
> +350 i386 init_module_from_fd sys_init_module_from_fd
The from_ seems redundant.
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 19439c7..5386629 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -860,4 +860,5 @@ asmlinkage long sys_process_vm_writev(pid_t pid,
>
> asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
> unsigned long idx1, unsigned long idx2);
> +asmlinkage long sys_init_module_from_fd(int len, const char __user *uargs);
> #endif
You mean, "int fd"?
> diff --git a/kernel/module.c b/kernel/module.c
> index 4edbd9c..b080cf8 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
...
> +/* Sets info->hdr and info->len. */
> +int copy_module_from_fd(int fd, struct load_info *info)
> +{
> + 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;
> +
> + if (stat.size > INT_MAX) {
> + err = -ENOMEM;
> + goto out;
> }
> + size = stat.size;
>
> - if (hdr->e_shoff >= len ||
> - hdr->e_shnum * sizeof(Elf_Shdr) > len - hdr->e_shoff) {
> - err = -ENOEXEC;
> - goto free_hdr;
> + info->hdr = vmalloc(size);
> + if (!info->hdr) {
> + err = -ENOMEM;
> + goto out;
The stat.size > INT_MAX is redundant: vmalloc is quite careful on its
checking of the size param.
(We removed a similar test from the module.c code a few years ago).
Cheers,
Rusty.
--
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/
