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@xxxxxxxxxxxx>
Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
---
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;
--
2.25.1
