As with the kernel_load_data LSM hook, add a "contents" flag to the kernel_read_file LSM hook that indicates whether the LSM can expect a matching call to the kernel_post_read_file LSM hook with the full contents of the file. With the coming addition of partial file read support for kernel_read_file*() API, the LSM will no longer be able to always see the entire contents of a file during the read calls.
For cases where the LSM must read examine the complete file contents, it will need to do so on its own every time the kernel_read_file hook is called with contents=false (or reject such cases). Adjust all existing LSMs to retain existing behavior.
Signed-off-by: Kees Cook keescook@chromium.org Reviewed-by: Mimi Zohar zohar@linux.ibm.com --- fs/kernel_read_file.c | 2 +- include/linux/ima.h | 6 ++++-- include/linux/lsm_hook_defs.h | 2 +- include/linux/lsm_hooks.h | 3 +++ include/linux/security.h | 6 ++++-- security/integrity/ima/ima_main.c | 10 +++++++++- security/loadpin/loadpin.c | 14 ++++++++++++-- security/security.c | 7 ++++--- security/selinux/hooks.c | 5 +++-- 9 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c index 2e29c38eb4df..d73bc3fa710a 100644 --- a/fs/kernel_read_file.c +++ b/fs/kernel_read_file.c @@ -39,7 +39,7 @@ int kernel_read_file(struct file *file, void **buf, if (ret) return ret;
- ret = security_kernel_read_file(file, id); + ret = security_kernel_read_file(file, id, true); if (ret) goto out;
diff --git a/include/linux/ima.h b/include/linux/ima.h index af9fb8c5f16a..8fa7bcfb2da2 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -23,7 +23,8 @@ extern int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot); extern int ima_load_data(enum kernel_load_data_id id, bool contents); extern int ima_post_load_data(char *buf, loff_t size, enum kernel_load_data_id id, char *description); -extern int ima_read_file(struct file *file, enum kernel_read_file_id id); +extern int ima_read_file(struct file *file, enum kernel_read_file_id id, + bool contents); extern int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id id); extern void ima_post_path_mknod(struct dentry *dentry); @@ -92,7 +93,8 @@ static inline int ima_post_load_data(char *buf, loff_t size, return 0; }
-static inline int ima_read_file(struct file *file, enum kernel_read_file_id id) +static inline int ima_read_file(struct file *file, enum kernel_read_file_id id, + bool contents) { return 0; } diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 83c6f1f5cc1e..d67cb3502310 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -188,7 +188,7 @@ LSM_HOOK(int, 0, kernel_load_data, enum kernel_load_data_id id, bool contents) LSM_HOOK(int, 0, kernel_post_load_data, char *buf, loff_t size, enum kernel_read_file_id id, char *description) LSM_HOOK(int, 0, kernel_read_file, struct file *file, - enum kernel_read_file_id id) + enum kernel_read_file_id id, bool contents) LSM_HOOK(int, 0, kernel_post_read_file, struct file *file, char *buf, loff_t size, enum kernel_read_file_id id) LSM_HOOK(int, 0, task_fix_setuid, struct cred *new, const struct cred *old, diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 6bb4f1a0158c..8814e3d5952d 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -651,6 +651,7 @@ * @file contains the file structure pointing to the file being read * by the kernel. * @id kernel read file identifier + * @contents if a subsequent @kernel_post_read_file will be called. * Return 0 if permission is granted. * @kernel_post_read_file: * Read a file specified by userspace. @@ -659,6 +660,8 @@ * @buf pointer to buffer containing the file contents. * @size length of the file contents. * @id kernel read file identifier + * This must be paired with a prior @kernel_read_file call that had + * @contents set to true. * Return 0 if permission is granted. * @task_fix_setuid: * Update the module's state after setting one or more of the user diff --git a/include/linux/security.h b/include/linux/security.h index 51c8e4e6b7cc..bc2725491560 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -391,7 +391,8 @@ int security_kernel_load_data(enum kernel_load_data_id id, bool contents); int security_kernel_post_load_data(char *buf, loff_t size, enum kernel_load_data_id id, char *description); -int security_kernel_read_file(struct file *file, enum kernel_read_file_id id); +int security_kernel_read_file(struct file *file, enum kernel_read_file_id id, + bool contents); int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, enum kernel_read_file_id id); int security_task_fix_setuid(struct cred *new, const struct cred *old, @@ -1030,7 +1031,8 @@ static inline int security_kernel_post_load_data(char *buf, loff_t size, }
static inline int security_kernel_read_file(struct file *file, - enum kernel_read_file_id id) + enum kernel_read_file_id id, + bool contents) { return 0; } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 6f2b8352573a..939f53d02627 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -602,6 +602,7 @@ void ima_post_path_mknod(struct dentry *dentry) * ima_read_file - pre-measure/appraise hook decision based on policy * @file: pointer to the file to be measured/appraised/audit * @read_id: caller identifier + * @contents: whether a subsequent call will be made to ima_post_read_file() * * Permit reading a file based on policy. The policy rules are written * in terms of the policy identifier. Appraising the integrity of @@ -609,8 +610,15 @@ void ima_post_path_mknod(struct dentry *dentry) * * For permission return 0, otherwise return -EACCES. */ -int ima_read_file(struct file *file, enum kernel_read_file_id read_id) +int ima_read_file(struct file *file, enum kernel_read_file_id read_id, + bool contents) { + /* Reject all partial reads during appraisal. */ + if (!contents) { + if (ima_appraise & IMA_APPRAISE_ENFORCE) + return -EACCES; + } + /* * Do devices using pre-allocated memory run the risk of the * firmware being accessible to the device prior to the completion diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c index 28782412febb..b12f7d986b1e 100644 --- a/security/loadpin/loadpin.c +++ b/security/loadpin/loadpin.c @@ -118,11 +118,21 @@ static void loadpin_sb_free_security(struct super_block *mnt_sb) } }
-static int loadpin_read_file(struct file *file, enum kernel_read_file_id id) +static int loadpin_read_file(struct file *file, enum kernel_read_file_id id, + bool contents) { struct super_block *load_root; const char *origin = kernel_read_file_id_str(id);
+ /* + * If we will not know that we'll be seeing the full contents + * then we cannot trust a load will be complete and unchanged + * off disk. Treat all contents=false hooks as if there were + * no associated file struct. + */ + if (!contents) + file = NULL; + /* If the file id is excluded, ignore the pinning. */ if ((unsigned int)id < ARRAY_SIZE(ignore_read_file_id) && ignore_read_file_id[id]) { @@ -179,7 +189,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
static int loadpin_load_data(enum kernel_load_data_id id, bool contents) { - return loadpin_read_file(NULL, (enum kernel_read_file_id) id); + return loadpin_read_file(NULL, (enum kernel_read_file_id) id, contents); }
static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = { diff --git a/security/security.c b/security/security.c index 531b855826fc..a28045dc9e7f 100644 --- a/security/security.c +++ b/security/security.c @@ -1672,14 +1672,15 @@ int security_kernel_module_request(char *kmod_name) return integrity_kernel_module_request(kmod_name); }
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id) +int security_kernel_read_file(struct file *file, enum kernel_read_file_id id, + bool contents) { int ret;
- ret = call_int_hook(kernel_read_file, 0, file, id); + ret = call_int_hook(kernel_read_file, 0, file, id, contents); if (ret) return ret; - return ima_read_file(file, id); + return ima_read_file(file, id, contents); } EXPORT_SYMBOL_GPL(security_kernel_read_file);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 558beee97d8d..dec654d52b68 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4003,13 +4003,14 @@ static int selinux_kernel_module_from_file(struct file *file) }
static int selinux_kernel_read_file(struct file *file, - enum kernel_read_file_id id) + enum kernel_read_file_id id, + bool contents) { int rc = 0;
switch (id) { case READING_MODULE: - rc = selinux_kernel_module_from_file(file); + rc = selinux_kernel_module_from_file(contents ? file : NULL); break; default: break;