ima_read_policy() loads a policy file with kernel_read_file_from_path()
and splits it into lines with
while (size > 0 && (p = strsep(&datap, "\n")))
kernel_read_file() allocates the destination with vmalloc(i_size) --
exactly i_size bytes, and writes no NUL terminator. strsep()'s scan for
the next '\n' is not bounded by @size, so when the last line has no
trailing newline the scan runs off the end of the buffer (CWE-125). When
i_size is a multiple of PAGE_SIZE the allocation has no slack and the
read walks into the vmalloc guard page and faults.
Reproduced under KASAN in a VM: writing the path of a page-aligned
policy file with no trailing newline to <securityfs>/ima/policy oopses:
BUG: unable to handle page fault for address: ffffc90000032000
#PF: supervisor read access in kernel mode
RIP: 0010:strsep+0x7a/0xd0
Call Trace:
ima_write_policy+0x1f4/0x260
vfs_write+0x16a/0x6f0
ksys_write+0xcb/0x160
do_syscall_64+0xe0/0x5a0
This requires CAP_MAC_ADMIN (the policy file is mode 0200), but a policy
file that does not end in a newline is an ordinary, non-malicious
condition, so a legitimate policy load can crash the kernel.
Walk the buffer with memchr() bounded by the remaining size instead of
strsep(): terminate each line in place at its newline, and parse a
NUL-terminated copy of a final line that has none. The explicit per-line
accounting replaces the old "size -= rc" step, whose off-by-one
(ima_parse_add_rule() returns strlen() + 1) made a trailing line without a
newline fail with -EINVAL; such a policy now loads. The loop now consumes
the buffer exactly, so the trailing "if (size) return -EINVAL" is dropped.
Fixes: 7429b092811f ("ima: load policy using path")
Assisted-by: copilot-cli:claude-opus-4-6 frama-c
Signed-off-by: Fabrice Derepas <[email protected]>
---
Tested under KASAN (CONFIG_KASAN_GENERIC + CONFIG_KASAN_VMALLOC) in QEMU,
loading a policy via "echo /path > <securityfs>/ima/policy":
- page-aligned file, no trailing newline: unpatched -> guard-page oops
in strsep()/ima_read_policy() (trace above); patched -> no fault, the
load fails cleanly with -EINVAL on the (garbage) content.
- valid policy with a trailing newline: loads before and after.
- valid rule with no trailing newline: unpatched -> -EINVAL (the size
underflow); patched -> loads.
lib/string.o is not KASAN-instrumented, so the over-read is caught by the
vmalloc guard page rather than a shadow report; the confirmation is the
page-fault oops with strsep()/ima_write_policy() in the trace.
security/integrity/ima/ima_fs.c | 46 ++++++++++++++++++++++++++-------
1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 174a94740..7b530b130 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -526,12 +526,10 @@ static const struct file_operations
ima_ascii_measurements_staged_ops = {
static ssize_t ima_read_policy(char *path)
{
void *data = NULL;
- char *datap;
- size_t size;
+ char *datap, *eol, *p;
+ size_t size, linelen;
int rc, pathlen = strlen(path);
- char *p;
-
/* remove \n */
datap = path;
strsep(&datap, "\n");
@@ -546,21 +544,49 @@ static ssize_t ima_read_policy(char *path)
rc = 0;
datap = data;
- while (size > 0 && (p = strsep(&datap, "\n"))) {
+ while (size > 0) {
+ eol = memchr(datap, '\n', size);
+ linelen = eol ? (size_t)(eol - datap) : size;
+
+ if (eol) {
+ /* NUL-terminate the line in place, within bounds. */
+ *eol = '\0';
+ p = datap;
+ } else {
+ /*
+ * kernel_read_file_from_path() does not NUL-terminate
+ * the buffer, and it may be exactly i_size bytes long,
+ * so a string walk off the end is possible. The final
+ * line without a trailing newline has no room for a
+ * terminator; parse a terminated copy instead.
+ */
+ p = kmemdup_nul(datap, linelen, GFP_KERNEL);
+ if (!p) {
+ rc = -ENOMEM;
+ break;
+ }
+ }
+
pr_debug("rule: %s\n", p);
rc = ima_parse_add_rule(p);
+ if (!eol)
+ kfree(p);
if (rc < 0)
break;
- size -= rc;
+ rc = 0;
+
+ datap += linelen;
+ size -= linelen;
+ if (eol) {
+ datap++; /* skip the newline */
+ size--;
+ }
}
vfree(data);
if (rc < 0)
return rc;
- else if (size)
- return -EINVAL;
- else
- return pathlen;
+ return pathlen;
}
static ssize_t ima_write_policy(struct file *file, const char __user *buf,
base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
--
2.53.0
