2021-12-22 16:44:24

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 02/19] ACPICA: Use original data_table_region pointer for accesses

From: Jessica Clarke <[email protected]>

ACPICA commit d9eb82bd7515989f0b29d79deeeb758db4d6529c

Currently the pointer to the table is cast to acpi_physical_address and
later cast back to a pointer to be dereferenced. Whether or not this is
supported is implementation-defined.

On CHERI, and thus Arm's experimental Morello prototype architecture,
pointers are represented as capabilities, which are unforgeable bounded
pointers, providing always-on fine-grained spatial memory safety. This
means that any pointer cast to a plain integer will lose all its
associated metadata, and when cast back to a pointer it will give a
null-derived pointer (one that has the same metadata as null but an
address equal to the integer) that will trap on any dereference. As a
result, this is an implementation where acpi_physical_address cannot be
used as a hack to store real pointers.

Thus, add a new field to struct acpi_object_region to store the pointer for
table regions, and propagate it to acpi_ex_data_table_space_handler via the
region context, to use a more portable implementation that supports
CHERI.

Link: https://github.com/acpica/acpica/commit/d9eb82bd
Signed-off-by: Bob Moore <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/acpica/acevents.h | 5 ++++
drivers/acpi/acpica/acobject.h | 1 +
drivers/acpi/acpica/dsopcode.c | 1 +
drivers/acpi/acpica/evhandler.c | 2 +-
drivers/acpi/acpica/evrgnini.c | 52 +++++++++++++++++++++++++++++++++
drivers/acpi/acpica/excreate.c | 1 +
drivers/acpi/acpica/exregion.c | 15 +++++++---
include/acpi/actypes.h | 4 +++
8 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/acpica/acevents.h b/drivers/acpi/acpica/acevents.h
index 82a75964343b..b29ba436944a 100644
--- a/drivers/acpi/acpica/acevents.h
+++ b/drivers/acpi/acpica/acevents.h
@@ -223,6 +223,11 @@ acpi_ev_pci_bar_region_setup(acpi_handle handle,
u32 function,
void *handler_context, void **region_context);

+acpi_status
+acpi_ev_data_table_region_setup(acpi_handle handle,
+ u32 function,
+ void *handler_context, void **region_context);
+
acpi_status
acpi_ev_default_region_setup(acpi_handle handle,
u32 function,
diff --git a/drivers/acpi/acpica/acobject.h b/drivers/acpi/acpica/acobject.h
index 9db5ae0f79ea..0aa0d847cb25 100644
--- a/drivers/acpi/acpica/acobject.h
+++ b/drivers/acpi/acpica/acobject.h
@@ -138,6 +138,7 @@ struct acpi_object_region {
union acpi_operand_object *next;
acpi_physical_address address;
u32 length;
+ void *pointer; /* Only for data table regions */
};

struct acpi_object_method {
diff --git a/drivers/acpi/acpica/dsopcode.c b/drivers/acpi/acpica/dsopcode.c
index 639635291ab7..44c448269861 100644
--- a/drivers/acpi/acpica/dsopcode.c
+++ b/drivers/acpi/acpica/dsopcode.c
@@ -531,6 +531,7 @@ acpi_ds_eval_table_region_operands(struct acpi_walk_state *walk_state,

obj_desc->region.address = ACPI_PTR_TO_PHYSADDR(table);
obj_desc->region.length = table->length;
+ obj_desc->region.pointer = table;

ACPI_DEBUG_PRINT((ACPI_DB_EXEC, "RgnObj %p Addr %8.8X%8.8X Len %X\n",
obj_desc,
diff --git a/drivers/acpi/acpica/evhandler.c b/drivers/acpi/acpica/evhandler.c
index c0cd7147a5a3..8f43d38dc4ca 100644
--- a/drivers/acpi/acpica/evhandler.c
+++ b/drivers/acpi/acpica/evhandler.c
@@ -386,7 +386,7 @@ acpi_ev_install_space_handler(struct acpi_namespace_node *node,
case ACPI_ADR_SPACE_DATA_TABLE:

handler = acpi_ex_data_table_space_handler;
- setup = NULL;
+ setup = acpi_ev_data_table_region_setup;
break;

default:
diff --git a/drivers/acpi/acpica/evrgnini.c b/drivers/acpi/acpica/evrgnini.c
index 984c172453bf..d28dee929e61 100644
--- a/drivers/acpi/acpica/evrgnini.c
+++ b/drivers/acpi/acpica/evrgnini.c
@@ -406,6 +406,58 @@ acpi_ev_cmos_region_setup(acpi_handle handle,
return_ACPI_STATUS(AE_OK);
}

+/*******************************************************************************
+ *
+ * FUNCTION: acpi_ev_data_table_region_setup
+ *
+ * PARAMETERS: handle - Region we are interested in
+ * function - Start or stop
+ * handler_context - Address space handler context
+ * region_context - Region specific context
+ *
+ * RETURN: Status
+ *
+ * DESCRIPTION: Setup a data_table_region
+ *
+ * MUTEX: Assumes namespace is not locked
+ *
+ ******************************************************************************/
+
+acpi_status
+acpi_ev_data_table_region_setup(acpi_handle handle,
+ u32 function,
+ void *handler_context, void **region_context)
+{
+ union acpi_operand_object *region_desc =
+ (union acpi_operand_object *)handle;
+ struct acpi_data_table_space_context *local_region_context;
+
+ ACPI_FUNCTION_TRACE(ev_data_table_region_setup);
+
+ if (function == ACPI_REGION_DEACTIVATE) {
+ if (*region_context) {
+ ACPI_FREE(*region_context);
+ *region_context = NULL;
+ }
+ return_ACPI_STATUS(AE_OK);
+ }
+
+ /* Create a new context */
+
+ local_region_context =
+ ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_data_table_space_context));
+ if (!(local_region_context)) {
+ return_ACPI_STATUS(AE_NO_MEMORY);
+ }
+
+ /* Save the data table pointer for use in the handler */
+
+ local_region_context->pointer = region_desc->region.pointer;
+
+ *region_context = local_region_context;
+ return_ACPI_STATUS(AE_OK);
+}
+
/*******************************************************************************
*
* FUNCTION: acpi_ev_default_region_setup
diff --git a/drivers/acpi/acpica/excreate.c b/drivers/acpi/acpica/excreate.c
index 80b52ad55775..deb3674ae726 100644
--- a/drivers/acpi/acpica/excreate.c
+++ b/drivers/acpi/acpica/excreate.c
@@ -279,6 +279,7 @@ acpi_ex_create_region(u8 * aml_start,
obj_desc->region.space_id = space_id;
obj_desc->region.address = 0;
obj_desc->region.length = 0;
+ obj_desc->region.pointer = NULL;
obj_desc->region.node = node;
obj_desc->region.handler = NULL;
obj_desc->common.flags &=
diff --git a/drivers/acpi/acpica/exregion.c b/drivers/acpi/acpica/exregion.c
index 82b713a9a193..48c19908fa4e 100644
--- a/drivers/acpi/acpica/exregion.c
+++ b/drivers/acpi/acpica/exregion.c
@@ -509,8 +509,15 @@ acpi_ex_data_table_space_handler(u32 function,
u64 *value,
void *handler_context, void *region_context)
{
+ struct acpi_data_table_space_context *mapping;
+ char *pointer;
+
ACPI_FUNCTION_TRACE(ex_data_table_space_handler);

+ mapping = (struct acpi_data_table_space_context *) region_context;
+ pointer = ACPI_CAST_PTR(char, mapping->pointer) +
+ (address - ACPI_PTR_TO_PHYSADDR(mapping->pointer));
+
/*
* Perform the memory read or write. The bit_width was already
* validated.
@@ -518,14 +525,14 @@ acpi_ex_data_table_space_handler(u32 function,
switch (function) {
case ACPI_READ:

- memcpy(ACPI_CAST_PTR(char, value),
- ACPI_PHYSADDR_TO_PTR(address), ACPI_DIV_8(bit_width));
+ memcpy(ACPI_CAST_PTR(char, value), pointer,
+ ACPI_DIV_8(bit_width));
break;

case ACPI_WRITE:

- memcpy(ACPI_PHYSADDR_TO_PTR(address),
- ACPI_CAST_PTR(char, value), ACPI_DIV_8(bit_width));
+ memcpy(pointer, ACPI_CAST_PTR(char, value),
+ ACPI_DIV_8(bit_width));
break;

default:
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index 248242dca28d..700c2449e85a 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -1221,6 +1221,10 @@ struct acpi_mem_space_context {
struct acpi_mem_mapping *first_mm;
};

+struct acpi_data_table_space_context {
+ void *pointer;
+};
+
/*
* struct acpi_memory_list is used only if the ACPICA local cache is enabled
*/
--
2.26.2