The commands for /proc/powerpc/ofdt that carry properties, "add_node",
"add_property" and "update_property", can make the parser read past the
buffer they were copied into:
(1) do_update_property() computes the end of the buffer after
parse_node() moved buf past the phandle. So the end is off by the
length of the phandle string, which can be padded with zeroes, and
parse_next_property() accepts a value that reaches that far past
the command. new_property() then copies those bytes into the
property, which everyone can read in /proc/device-tree.
(2) parse_next_property() parses the length with simple_strtoul() into
an int and only rejects -1. A length above INT_MAX turns negative,
tmp += *length moves backwards and passes the check against the
end, so *tmp is read up to 2G in front of the buffer.
(3) Updating "slb-size" or "ibm,slb-size" reads an int from the value
whatever its length.
Compute the end before parsing the phandle, reject lengths that don't
fit into an int or into what is left of the buffer, and only take the
slb size from a value that holds one. The file is root-only and
refused under lockdown but the parser still shouldn't read past its
input.
Reported-by: Chris Mason <mason@kernel.org>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 610d91511f99 ("[PATCH] powerpc: Add support for changing properties from userspace")
Fixes: 46db2f86a3b2 ("powerpc/pseries: Fix to handle slb resize across migration")
Cc: <redacted>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
arch/powerpc/platforms/pseries/reconfig.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 18e3f1a036e3..b41c5db726c9 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -114,6 +114,7 @@ static void release_prop_list(const struct property *prop)
static char * parse_next_property(char *buf, char *end, char **name, int *length,
unsigned char **value)
{
+ unsigned long len;
char *tmp;
*name = buf;@@ -133,13 +134,13 @@ static char * parse_next_property(char *buf, char *end, char **name, int *length
}
/* now we're on the length */
- *length = -1;
- *length = simple_strtoul(tmp, &tmp, 10);
- if (*length == -1) {
+ len = simple_strtoul(tmp, &tmp, 10);
+ if (len > INT_MAX) {
printk(KERN_ERR "property parse failed in %s at line %d\n",
__func__, __LINE__);
return NULL;
}
+ *length = len;
if (*tmp != ' ' || ++tmp >= end) {
printk(KERN_ERR "property parse failed in %s at line %d\n",
__func__, __LINE__);@@ -148,13 +149,13 @@ static char * parse_next_property(char *buf, char *end, char **name, int *length
/* now we're on the value */
*value = tmp;
- tmp += *length;
- if (tmp > end) {
+ if (len > end - tmp) {
printk(KERN_ERR "property parse failed in %s at line %d\n",
__func__, __LINE__);
return NULL;
}
- else if (tmp < end && *tmp != ' ' && *tmp != '\0') {
+ tmp += len;
+ if (tmp < end && *tmp != ' ' && *tmp != '\0') {
printk(KERN_ERR "property parse failed in %s at line %d\n",
__func__, __LINE__);
return NULL;@@ -320,8 +321,8 @@ static int do_update_property(char *buf, size_t bufsize)
char *name, *end, *next_prop;
int length;
struct property *newprop;
- buf = parse_node(buf, bufsize, &np);
end = buf + bufsize;
+ buf = parse_node(buf, bufsize, &np);
if (!np)
return -ENODEV;
@@ -337,7 +338,8 @@ static int do_update_property(char *buf, size_t bufsize)
if (!newprop)
return -ENOMEM;
- if (!strcmp(name, "slb-size") || !strcmp(name, "ibm,slb-size"))
+ if ((!strcmp(name, "slb-size") || !strcmp(name, "ibm,slb-size")) &&
+ length >= (int)sizeof(int))
slb_set_size(*(int *)value);
return of_update_property(np, newprop);
--
2.53.0