[PATCH v3 01/14] x86/boot: Fix memremap of setup_indirect structures

From: Ross Philipson
Date: Mon Aug 09 2021 - 12:32:04 EST

Next message: Ross Philipson: "[PATCH v3 00/14] x86: Trenchboot secure dynamic launch Linux kernel support"
Previous message: Ross Philipson: "[PATCH v3 11/14] kexec: Secure Launch kexec SEXIT support"
In reply to: Ross Philipson: "[PATCH v3 11/14] kexec: Secure Launch kexec SEXIT support"
Next in thread: Ross Philipson: "[PATCH v3 04/14] Documentation/x86: Secure Launch kernel documentation"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

As documented, the setup_indirect structure is nested inside
the setup_data structures in the setup_data list. The code was
accessing the fields inside the setup_indirect structure but only
the sizeof(struct setup_data) was being memremapped. No crash
occured but this is just due to how the area was remapped under the
covers.

The setup_indirect structure was introduced in commit:

commit b3c72fc9a78e (x86/boot: Introduce setup_indirect)

Signed-off-by: Ross Philipson <ross.philipson@xxxxxxxxxx>
---
arch/x86/kernel/e820.c | 31 ++++++++++++++++---------
arch/x86/kernel/kdebugfs.c | 28 ++++++++++++++++-------
arch/x86/kernel/ksysfs.c | 56 ++++++++++++++++++++++++++++++++++++----------
arch/x86/kernel/setup.c | 23 +++++++++++++------
arch/x86/mm/ioremap.c | 13 +++++++----
5 files changed, 109 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index bc0657f..e023950 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -996,7 +996,8 @@ static int __init parse_memmap_opt(char *str)
void __init e820__reserve_setup_data(void)
{
struct setup_data *data;
- u64 pa_data;
+ u64 pa_data, pa_next;
+ u32 len;

pa_data = boot_params.hdr.setup_data;
if (!pa_data)
@@ -1004,6 +1005,9 @@ void __init e820__reserve_setup_data(void)

while (pa_data) {
data = early_memremap(pa_data, sizeof(*data));
+ len = sizeof(*data);
+ pa_next = data->next;
+
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);

/*
@@ -1015,18 +1019,23 @@ void __init e820__reserve_setup_data(void)
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);

- if (data->type == SETUP_INDIRECT &&
- ((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
- e820__range_update(((struct setup_indirect *)data->data)->addr,
- ((struct setup_indirect *)data->data)->len,
- E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
- e820__range_update_kexec(((struct setup_indirect *)data->data)->addr,
- ((struct setup_indirect *)data->data)->len,
- E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
+ if (data->type == SETUP_INDIRECT) {
+ len += data->len;
+ early_memunmap(data, sizeof(*data));
+ data = early_memremap(pa_data, len);
+
+ if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
+ e820__range_update(((struct setup_indirect *)data->data)->addr,
+ ((struct setup_indirect *)data->data)->len,
+ E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
+ e820__range_update_kexec(((struct setup_indirect *)data->data)->addr,
+ ((struct setup_indirect *)data->data)->len,
+ E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
+ }
}

- pa_data = data->next;
- early_memunmap(data, sizeof(*data));
+ pa_data = pa_next;
+ early_memunmap(data, len);
}

e820__update_table(e820_table);
diff --git a/arch/x86/kernel/kdebugfs.c b/arch/x86/kernel/kdebugfs.c
index 64b6da9..2fa1b93 100644
--- a/arch/x86/kernel/kdebugfs.c
+++ b/arch/x86/kernel/kdebugfs.c
@@ -92,7 +92,8 @@ static int __init create_setup_data_nodes(struct dentry *parent)
struct setup_data *data;
int error;
struct dentry *d;
- u64 pa_data;
+ u64 pa_data, pa_next;
+ u32 len;
int no = 0;

d = debugfs_create_dir("setup_data", parent);
@@ -112,12 +113,23 @@ static int __init create_setup_data_nodes(struct dentry *parent)
error = -ENOMEM;
goto err_dir;
}
-
- if (data->type == SETUP_INDIRECT &&
- ((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
- node->paddr = ((struct setup_indirect *)data->data)->addr;
- node->type = ((struct setup_indirect *)data->data)->type;
- node->len = ((struct setup_indirect *)data->data)->len;
+ pa_next = data->next;
+
+ if (data->type == SETUP_INDIRECT) {
+ len = sizeof(*data) + data->len;
+ memunmap(data);
+ data = memremap(pa_data, len, MEMREMAP_WB);
+ if (!data) {
+ kfree(node);
+ error = -ENOMEM;
+ goto err_dir;
+ }
+
+ if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
+ node->paddr = ((struct setup_indirect *)data->data)->addr;
+ node->type = ((struct setup_indirect *)data->data)->type;
+ node->len = ((struct setup_indirect *)data->data)->len;
+ }
} else {
node->paddr = pa_data;
node->type = data->type;
@@ -125,7 +137,7 @@ static int __init create_setup_data_nodes(struct dentry *parent)
}

create_setup_data_node(d, no, node);
- pa_data = data->next;
+ pa_data = pa_next;

memunmap(data);
no++;
diff --git a/arch/x86/kernel/ksysfs.c b/arch/x86/kernel/ksysfs.c
index d0a1912..4cef401 100644
--- a/arch/x86/kernel/ksysfs.c
+++ b/arch/x86/kernel/ksysfs.c
@@ -93,24 +93,35 @@ static int __init get_setup_data_size(int nr, size_t *size)
{
int i = 0;
struct setup_data *data;
- u64 pa_data = boot_params.hdr.setup_data;
+ u64 pa_data = boot_params.hdr.setup_data, pa_next;
+ u32 len;

while (pa_data) {
data = memremap(pa_data, sizeof(*data), MEMREMAP_WB);
if (!data)
return -ENOMEM;
+ pa_next = data->next;
+
if (nr == i) {
- if (data->type == SETUP_INDIRECT &&
- ((struct setup_indirect *)data->data)->type != SETUP_INDIRECT)
- *size = ((struct setup_indirect *)data->data)->len;
- else
+ if (data->type == SETUP_INDIRECT) {
+ len = sizeof(*data) + data->len;
+ memunmap(data);
+ data = memremap(pa_data, len, MEMREMAP_WB);
+ if (!data)
+ return -ENOMEM;
+
+ if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT)
+ *size = ((struct setup_indirect *)data->data)->len;
+ else
+ *size = data->len;
+ } else
*size = data->len;

memunmap(data);
return 0;
}

- pa_data = data->next;
+ pa_data = pa_next;
memunmap(data);
i++;
}
@@ -122,6 +133,7 @@ static ssize_t type_show(struct kobject *kobj,
{
int nr, ret;
u64 paddr;
+ u32 len;
struct setup_data *data;

ret = kobj_to_setup_data_nr(kobj, &nr);
@@ -135,9 +147,14 @@ static ssize_t type_show(struct kobject *kobj,
if (!data)
return -ENOMEM;

- if (data->type == SETUP_INDIRECT)
+ if (data->type == SETUP_INDIRECT) {
+ len = sizeof(*data) + data->len;
+ memunmap(data);
+ data = memremap(paddr, len, MEMREMAP_WB);
+ if (!data)
+ return -ENOMEM;
ret = sprintf(buf, "0x%x\n", ((struct setup_indirect *)data->data)->type);
- else
+ } else
ret = sprintf(buf, "0x%x\n", data->type);
memunmap(data);
return ret;
@@ -165,10 +182,25 @@ static ssize_t setup_data_data_read(struct file *fp,
if (!data)
return -ENOMEM;

- if (data->type == SETUP_INDIRECT &&
- ((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
- paddr = ((struct setup_indirect *)data->data)->addr;
- len = ((struct setup_indirect *)data->data)->len;
+ if (data->type == SETUP_INDIRECT) {
+ len = sizeof(*data) + data->len;
+ memunmap(data);
+ data = memremap(paddr, len, MEMREMAP_WB);
+ if (!data)
+ return -ENOMEM;
+
+ if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
+ paddr = ((struct setup_indirect *)data->data)->addr;
+ len = ((struct setup_indirect *)data->data)->len;
+ } else {
+ /*
+ * Even though this is technically undefined, return
+ * the data as though it is a normal setup_data struct.
+ * This will at least allow it to be inspected.
+ */
+ paddr += sizeof(*data);
+ len = data->len;
+ }
} else {
paddr += sizeof(*data);
len = data->len;
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index bff3a78..055a834 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -368,20 +368,29 @@ static void __init parse_setup_data(void)
static void __init memblock_x86_reserve_range_setup_data(void)
{
struct setup_data *data;
- u64 pa_data;
+ u64 pa_data, pa_next;
+ u32 len;

pa_data = boot_params.hdr.setup_data;
while (pa_data) {
data = early_memremap(pa_data, sizeof(*data));
+ len = sizeof(*data);
+ pa_next = data->next;
+
memblock_reserve(pa_data, sizeof(*data) + data->len);

- if (data->type == SETUP_INDIRECT &&
- ((struct setup_indirect *)data->data)->type != SETUP_INDIRECT)
- memblock_reserve(((struct setup_indirect *)data->data)->addr,
- ((struct setup_indirect *)data->data)->len);
+ if (data->type == SETUP_INDIRECT) {
+ len += data->len;
+ early_memunmap(data, sizeof(*data));
+ data = early_memremap(pa_data, len);

- pa_data = data->next;
- early_memunmap(data, sizeof(*data));
+ if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT)
+ memblock_reserve(((struct setup_indirect *)data->data)->addr,
+ ((struct setup_indirect *)data->data)->len);
+ }
+
+ pa_data = pa_next;
+ early_memunmap(data, len);
}
}

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 60ade7d..ab74e4f 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -635,10 +635,15 @@ static bool memremap_is_setup_data(resource_size_t phys_addr,
return true;
}

- if (data->type == SETUP_INDIRECT &&
- ((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
- paddr = ((struct setup_indirect *)data->data)->addr;
- len = ((struct setup_indirect *)data->data)->len;
+ if (data->type == SETUP_INDIRECT) {
+ memunmap(data);
+ data = memremap(paddr, sizeof(*data) + len,
+ MEMREMAP_WB | MEMREMAP_DEC);
+
+ if (((struct setup_indirect *)data->data)->type != SETUP_INDIRECT) {
+ paddr = ((struct setup_indirect *)data->data)->addr;
+ len = ((struct setup_indirect *)data->data)->len;
+ }
}

memunmap(data);
--
1.8.3.1

Next message: Ross Philipson: "[PATCH v3 00/14] x86: Trenchboot secure dynamic launch Linux kernel support"
Previous message: Ross Philipson: "[PATCH v3 11/14] kexec: Secure Launch kexec SEXIT support"
In reply to: Ross Philipson: "[PATCH v3 11/14] kexec: Secure Launch kexec SEXIT support"
Next in thread: Ross Philipson: "[PATCH v3 04/14] Documentation/x86: Secure Launch kernel documentation"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]