[PATCH] ima: bound line scan in ima_read_policy() to fix OOB read

[PATCH] ima: bound line scan in ima_read_policy() to fix OOB read

[Fabrice Derepas]

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 <fabrice.derepas@canonical.com>
---
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

Re: [PATCH] ima: bound line scan in ima_read_policy() to fix OOB read

[Roberto Sassu]

On Fri, 2026-08-14 at 10:54 +0200, Fabrice Derepas wrote:
> 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.

Looks unnecessarily complicated. I would replicate instead the same
behavior of ima_write_policy() to allocate a buffer with an additional
byte for the terminator.

Read the inode size, vmalloc() size + 1, set the terminator, and pass
the buffer to kernel_read_file_from_path().

I would still pass NULL for file_size to save ourselves from rechecking
if it changed after the kernel_read_file_from_path() call.

Thanks

Roberto

> Fixes: 7429b092811f ("ima: load policy using path")
> Assisted-by: copilot-cli:claude-opus-4-6 frama-c
> Signed-off-by: Fabrice Derepas <fabrice.derepas@canonical.com>
> ---
> 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

Re: [PATCH] ima: bound line scan in ima_read_policy() to fix OOB read

[Roberto Sassu]

On Wed, 2026-08-26 at 11:51 +0200, Roberto Sassu wrote:
> On Fri, 2026-08-14 at 10:54 +0200, Fabrice Derepas wrote:
> > 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.
> 
> Looks unnecessarily complicated. I would replicate instead the same
> behavior of ima_write_policy() to allocate a buffer with an additional
> byte for the terminator.
> 
> Read the inode size, vmalloc() size + 1, set the terminator, and pass
> the buffer to kernel_read_file_from_path().
> 
> I would still pass NULL for file_size to save ourselves from rechecking
> if it changed after the kernel_read_file_from_path() call.

Could be simpler...

Something like this? (not tested)

diff --git a/security/integrity/ima/ima_fs.c
b/security/integrity/ima/ima_fs.c
index 3b8174a91a34..6151f440cd2f 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -528,7 +528,7 @@ static const struct file_operations
ima_ascii_measurements_staged_ops = {
 static ssize_t ima_read_policy(char *path)
 {
 	void *data = NULL;
-	char *datap;
+	char *datap, *buf = NULL;
 	size_t size;
 	int rc, pathlen = strlen(path);
 
@@ -548,6 +548,17 @@ static ssize_t ima_read_policy(char *path)
 	rc = 0;
 
 	datap = data;
+	if (size && datap[size - 1] != '\n') {
+		buf = kmemdup_nul(data, size, GFP_KERNEL);
+		if (!buf) {
+			vfree(data);
+			return -ENOMEM;
+		}
+		datap = buf;
+		/* Count the NUL terminator. */
+		size++;
+	}
+
 	while (size > 0 && (p = strsep(&datap, "\n"))) {
 		pr_debug("rule: %s\n", p);
 		rc = ima_parse_add_rule(p);
@@ -557,6 +568,7 @@ static ssize_t ima_read_policy(char *path)
 	}
 
 	vfree(data);
+	kfree(buf);
 	if (rc < 0)
 		return rc;
 	else if (size)

Roberto

> 
> Thanks
> 
> Roberto
> 
> > Fixes: 7429b092811f ("ima: load policy using path")
> > Assisted-by: copilot-cli:claude-opus-4-6 frama-c
> > Signed-off-by: Fabrice Derepas <fabrice.derepas@canonical.com>
> > ---
> > 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
> 

[PATCH] ima: Fix out-of-bounds read in ima_read_policy()

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

ima_read_policy() reads the policy file with kernel_read_file_from_path()
and splits it into rules with strsep(). However, the buffer allocated by
kernel_read_file_from_path() is exactly the size of the file and is not
NUL-terminated. If the policy file does not end with a newline, strsep()
scans past the end of the buffer while looking for the next separator. If
the file size is a multiple of PAGE_SIZE, the scan reaches the vmalloc
guard page and the kernel oopses.

If the last byte of the policy is not a newline, parse a NUL-terminated
copy of the buffer obtained with kmemdup_nul() instead, and account for
the NUL terminator in the remaining size, since ima_parse_add_rule()
counts it in the length of the last rule.

As a side effect, a policy whose last rule is not followed by a newline
is now loaded, instead of being rejected with -EINVAL due to the
terminator being counted in the length of the last rule but not in the
file size.

Fixes: 7429b092811f ("ima: load policy using path")
Reported-by: Fabrice Derepas <fabrice.derepas@canonical.com>
Closes: https://lore.kernel.org/linux-integrity/20260814085443.1211989-1-fabrice.derepas@canonical.com/
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_fs.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 2a0bca554316..6d5e061f88c2 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -528,7 +528,7 @@ static const struct file_operations ima_ascii_measurements_staged_ops = {
 static ssize_t ima_read_policy(char *path)
 {
 	void *data = NULL;
-	char *datap;
+	char *datap, *buf = NULL;
 	size_t size;
 	int rc, pathlen = strlen(path);
 
@@ -548,6 +548,23 @@ static ssize_t ima_read_policy(char *path)
 	rc = 0;
 
 	datap = data;
+
+	/*
+	 * kernel_read_file_from_path() does not NUL-terminate the buffer.
+	 * If the policy does not end with a newline, parse a NUL-terminated
+	 * copy, so that strsep() does not scan past the end of the buffer.
+	 */
+	if (size && datap[size - 1] != '\n') {
+		buf = kmemdup_nul(data, size, GFP_KERNEL);
+		if (!buf) {
+			vfree(data);
+			return -ENOMEM;
+		}
+		datap = buf;
+		/* Count the NUL terminator, included in ima_parse_add_rule(). */
+		size++;
+	}
+
 	while (size > 0 && (p = strsep(&datap, "\n"))) {
 		pr_debug("rule: %s\n", p);
 		rc = ima_parse_add_rule(p);
@@ -557,6 +574,7 @@ static ssize_t ima_read_policy(char *path)
 	}
 
 	vfree(data);
+	kfree(buf);
 	if (rc < 0)
 		return rc;
 	else if (size)
-- 
2.43.0

Re: [PATCH] ima: Fix out-of-bounds read in ima_read_policy()

[sashiko-bot]

> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> ima_read_policy() reads the policy file with kernel_read_file_from_path()
> and splits it into rules with strsep(). However, the buffer allocated by
> kernel_read_file_from_path() is exactly the size of the file and is not
> NUL-terminated. If the policy file does not end with a newline, strsep()
> scans past the end of the buffer while looking for the next separator. If
> the file size is a multiple of PAGE_SIZE, the scan reaches the vmalloc
> guard page and the kernel oopses.
> 
> If the last byte of the policy is not a newline, parse a NUL-terminated
> copy of the buffer obtained with kmemdup_nul() instead, and account for
> the NUL terminator in the remaining size, since ima_parse_add_rule()
> counts it in the length of the last rule.
> 
> As a side effect, a policy whose last rule is not followed by a newline
> is now loaded, instead of being rejected with -EINVAL due to the
> terminator being counted in the length of the last rule but not in the
> file size.
> 
> Fixes: 7429b092811f ("ima: load policy using path")
> Reported-by: Fabrice Derepas <fabrice.derepas@canonical.com>
> Closes: https://lore.kernel.org/linux-integrity/20260814085443.1211989-1-fabrice.derepas@canonical.com/
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112143029.4159022-1-roberto.sassu@huaweicloud.com?part=1