x86/boot: Fix memremap of setup_indirect structures
authorRoss Philipson <ross.philipson@oracle.com>
Thu, 24 Feb 2022 02:07:35 +0000 (21:07 -0500)
committerBorislav Petkov <bp@suse.de>
Wed, 9 Mar 2022 11:49:44 +0000 (12:49 +0100)
As documented, the setup_indirect structure is nested inside
the setup_data structures in the setup_data list. The code currently
accesses the fields inside the setup_indirect structure but only
the sizeof(struct setup_data) is being memremapped. No crash
occurred but this is just due to how the area is remapped under the
covers.

Properly memremap both the setup_data and setup_indirect structures
in these cases before accessing them.

Fixes: b3c72fc9a78e ("x86/boot: Introduce setup_indirect")
Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/1645668456-22036-2-git-send-email-ross.philipson@oracle.com
arch/x86/kernel/e820.c
arch/x86/kernel/kdebugfs.c
arch/x86/kernel/ksysfs.c
arch/x86/kernel/setup.c
arch/x86/mm/ioremap.c

index bc0657f0deedf2b13641088b9b8ea9173262f70b..f267205f2d5a419e68b66c4053b6cbdedb4273ef 100644 (file)
@@ -995,8 +995,10 @@ early_param("memmap", parse_memmap_opt);
  */
 void __init e820__reserve_setup_data(void)
 {
+       struct setup_indirect *indirect;
        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 +1006,14 @@ void __init e820__reserve_setup_data(void)
 
        while (pa_data) {
                data = early_memremap(pa_data, sizeof(*data));
+               if (!data) {
+                       pr_warn("e820: failed to memremap setup_data entry\n");
+                       return;
+               }
+
+               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 +1025,27 @@ 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 (!data) {
+                               pr_warn("e820: failed to memremap indirect setup_data\n");
+                               return;
+                       }
+
+                       indirect = (struct setup_indirect *)data->data;
+
+                       if (indirect->type != SETUP_INDIRECT) {
+                               e820__range_update(indirect->addr, indirect->len,
+                                                  E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
+                               e820__range_update_kexec(indirect->addr, indirect->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);
index 64b6da95af984868962777bb4978b3fa8a4eff16..e2e89bebcbc32840357788cbd803805e5f652c62 100644 (file)
@@ -88,11 +88,13 @@ create_setup_data_node(struct dentry *parent, int no,
 
 static int __init create_setup_data_nodes(struct dentry *parent)
 {
+       struct setup_indirect *indirect;
        struct setup_data_node *node;
        struct setup_data *data;
-       int error;
+       u64 pa_data, pa_next;
        struct dentry *d;
-       u64 pa_data;
+       int error;
+       u32 len;
        int no = 0;
 
        d = debugfs_create_dir("setup_data", parent);
@@ -112,12 +114,29 @@ 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;
+                       }
+
+                       indirect = (struct setup_indirect *)data->data;
+
+                       if (indirect->type != SETUP_INDIRECT) {
+                               node->paddr = indirect->addr;
+                               node->type  = indirect->type;
+                               node->len   = indirect->len;
+                       } else {
+                               node->paddr = pa_data;
+                               node->type  = data->type;
+                               node->len   = data->len;
+                       }
                } else {
                        node->paddr = pa_data;
                        node->type  = data->type;
@@ -125,7 +144,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++;
index d0a19121c6a4f1f4bb1f62e05f12d8350d4710a1..257892fcefa794803d8eaf2d3d1810ebb278957b 100644 (file)
@@ -91,26 +91,41 @@ static int get_setup_data_paddr(int nr, u64 *paddr)
 
 static int __init get_setup_data_size(int nr, size_t *size)
 {
-       int i = 0;
+       u64 pa_data = boot_params.hdr.setup_data, pa_next;
+       struct setup_indirect *indirect;
        struct setup_data *data;
-       u64 pa_data = boot_params.hdr.setup_data;
+       int i = 0;
+       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;
+
+                               indirect = (struct setup_indirect *)data->data;
+
+                               if (indirect->type != SETUP_INDIRECT)
+                                       *size = indirect->len;
+                               else
+                                       *size = data->len;
+                       } else {
                                *size = data->len;
+                       }
 
                        memunmap(data);
                        return 0;
                }
 
-               pa_data = data->next;
+               pa_data = pa_next;
                memunmap(data);
                i++;
        }
@@ -120,9 +135,11 @@ static int __init get_setup_data_size(int nr, size_t *size)
 static ssize_t type_show(struct kobject *kobj,
                         struct kobj_attribute *attr, char *buf)
 {
+       struct setup_indirect *indirect;
+       struct setup_data *data;
        int nr, ret;
        u64 paddr;
-       struct setup_data *data;
+       u32 len;
 
        ret = kobj_to_setup_data_nr(kobj, &nr);
        if (ret)
@@ -135,10 +152,20 @@ static ssize_t type_show(struct kobject *kobj,
        if (!data)
                return -ENOMEM;
 
-       if (data->type == SETUP_INDIRECT)
-               ret = sprintf(buf, "0x%x\n", ((struct setup_indirect *)data->data)->type);
-       else
+       if (data->type == SETUP_INDIRECT) {
+               len = sizeof(*data) + data->len;
+               memunmap(data);
+               data = memremap(paddr, len, MEMREMAP_WB);
+               if (!data)
+                       return -ENOMEM;
+
+               indirect = (struct setup_indirect *)data->data;
+
+               ret = sprintf(buf, "0x%x\n", indirect->type);
+       } else {
                ret = sprintf(buf, "0x%x\n", data->type);
+       }
+
        memunmap(data);
        return ret;
 }
@@ -149,9 +176,10 @@ static ssize_t setup_data_data_read(struct file *fp,
                                    char *buf,
                                    loff_t off, size_t count)
 {
+       struct setup_indirect *indirect;
+       struct setup_data *data;
        int nr, ret = 0;
        u64 paddr, len;
-       struct setup_data *data;
        void *p;
 
        ret = kobj_to_setup_data_nr(kobj, &nr);
@@ -165,10 +193,27 @@ 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;
+
+               indirect = (struct setup_indirect *)data->data;
+
+               if (indirect->type != SETUP_INDIRECT) {
+                       paddr = indirect->addr;
+                       len = indirect->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;
index f7a132eb794d8f12ca916879976aaf70345d4041..90d7e1788c91d25e3ad7538bc5ac0c5351c94b91 100644 (file)
@@ -369,21 +369,41 @@ static void __init parse_setup_data(void)
 
 static void __init memblock_x86_reserve_range_setup_data(void)
 {
+       struct setup_indirect *indirect;
        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));
+               if (!data) {
+                       pr_warn("setup: failed to memremap setup_data entry\n");
+                       return;
+               }
+
+               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);
+                       if (!data) {
+                               pr_warn("setup: failed to memremap indirect setup_data\n");
+                               return;
+                       }
 
-               pa_data = data->next;
-               early_memunmap(data, sizeof(*data));
+                       indirect = (struct setup_indirect *)data->data;
+
+                       if (indirect->type != SETUP_INDIRECT)
+                               memblock_reserve(indirect->addr, indirect->len);
+               }
+
+               pa_data = pa_next;
+               early_memunmap(data, len);
        }
 }
 
index 026031b3b7829517ae0e04295f3c10e430c3cd00..ab666c4cd357b037c2b6e120e933e7facd85f94d 100644 (file)
@@ -615,6 +615,7 @@ static bool memremap_is_efi_data(resource_size_t phys_addr,
 static bool memremap_is_setup_data(resource_size_t phys_addr,
                                   unsigned long size)
 {
+       struct setup_indirect *indirect;
        struct setup_data *data;
        u64 paddr, paddr_next;
 
@@ -627,6 +628,10 @@ static bool memremap_is_setup_data(resource_size_t phys_addr,
 
                data = memremap(paddr, sizeof(*data),
                                MEMREMAP_WB | MEMREMAP_DEC);
+               if (!data) {
+                       pr_warn("failed to memremap setup_data entry\n");
+                       return false;
+               }
 
                paddr_next = data->next;
                len = data->len;
@@ -636,10 +641,21 @@ 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 (!data) {
+                               pr_warn("failed to memremap indirect setup_data\n");
+                               return false;
+                       }
+
+                       indirect = (struct setup_indirect *)data->data;
+
+                       if (indirect->type != SETUP_INDIRECT) {
+                               paddr = indirect->addr;
+                               len = indirect->len;
+                       }
                }
 
                memunmap(data);