[Skiboot] [PATCH 2/2] Use additional checks in skiboot for pointers

Balbir Singh bsingharora at gmail.com
Mon Jul 18 13:39:16 AEST 2016


The checks validate pointers sent in using
opal_addr_valid() in opal_call API's provided
via the console, cpu, fdt, flash, i2c, interrupts,
nvram, opal-msg, opal, opal-pci, and
cec modules

xscom_read is ommitted from the checks as its used
internally as well and top_of_ram is not known until
mem_region_init

I've tested the changes on an actual machine

Signed-off-by: Balbir Singh <bsingharora at gmail.com>
---
 core/console.c      | 12 +++++++++
 core/cpu.c          |  6 +++++
 core/fdt.c          |  3 +++
 core/flash.c        |  6 +++++
 core/i2c.c          |  3 +++
 core/interrupts.c   |  6 +++++
 core/nvram.c        |  8 ++++++
 core/opal-msg.c     |  6 +++++
 core/opal.c         |  4 +++
 core/pci-opal.c     | 70 +++++++++++++++++++++++++++++++++++++++++++++++------
 core/test/run-msg.c |  3 +++
 hw/cec.c            |  3 +++
 12 files changed, 123 insertions(+), 7 deletions(-)

diff --git a/core/console.c b/core/console.c
index e851fcf..3190a7f 100644
--- a/core/console.c
+++ b/core/console.c
@@ -338,6 +338,10 @@ static int64_t dummy_console_write(int64_t term_number, int64_t *length,
 {
 	if (term_number != 0)
 		return OPAL_PARAMETER;
+
+	if (!opal_addr_valid(length) || !opal_addr_valid(buffer))
+		return OPAL_PARAMETER;
+
 	write(0, buffer, *length);
 
 	return OPAL_SUCCESS;
@@ -349,6 +353,10 @@ static int64_t dummy_console_write_buffer_space(int64_t term_number,
 {
 	if (term_number != 0)
 		return OPAL_PARAMETER;
+
+	if (!opal_addr_valid(length))
+		return OPAL_PARAMETER;
+
 	if (length)
 		*length = INMEM_CON_OUT_LEN;
 
@@ -361,6 +369,10 @@ static int64_t dummy_console_read(int64_t term_number, int64_t *length,
 {
 	if (term_number != 0)
 		return OPAL_PARAMETER;
+
+	if (!opal_addr_valid(length) || !opal_addr_valid(buffer))
+		return OPAL_PARAMETER;
+
 	*length = read(0, buffer, *length);
 	opal_update_pending_evt(OPAL_EVENT_CONSOLE_INPUT, 0);
 
diff --git a/core/cpu.c b/core/cpu.c
index f33ac48..688a0c1 100644
--- a/core/cpu.c
+++ b/core/cpu.c
@@ -712,6 +712,9 @@ static int64_t opal_start_cpu_thread(uint64_t server_no, uint64_t start_address)
 	struct cpu_thread *cpu;
 	struct cpu_job *job;
 
+	if (!opal_addr_valid((void *)start_address))
+		return OPAL_PARAMETER;
+
 	cpu = find_cpu_by_server(server_no);
 	if (!cpu) {
 		prerror("OPAL: Start invalid CPU 0x%04llx !\n", server_no);
@@ -747,6 +750,9 @@ static int64_t opal_query_cpu_status(uint64_t server_no, uint8_t *thread_status)
 {
 	struct cpu_thread *cpu;
 
+	if (!opal_addr_valid(thread_status))
+		return OPAL_PARAMETER;
+
 	cpu = find_cpu_by_server(server_no);
 	if (!cpu) {
 		prerror("OPAL: Query invalid CPU 0x%04llx !\n", server_no);
diff --git a/core/fdt.c b/core/fdt.c
index a22a840..5bdb83c 100644
--- a/core/fdt.c
+++ b/core/fdt.c
@@ -226,6 +226,9 @@ static int64_t opal_get_device_tree(uint32_t phandle,
 	int64_t totalsize;
 	int ret;
 
+	if (!opal_addr_valid(fdt))
+		return OPAL_PARAMETER;
+
 	root = dt_find_by_phandle(dt_root, phandle);
 	if (!root)
 		return OPAL_PARAMETER;
diff --git a/core/flash.c b/core/flash.c
index 5036707..6d75744 100644
--- a/core/flash.c
+++ b/core/flash.c
@@ -385,12 +385,18 @@ err:
 static int64_t opal_flash_read(uint64_t id, uint64_t offset, uint64_t buf,
 		uint64_t size, uint64_t token)
 {
+	if (!opal_addr_valid((void *)buf))
+		return OPAL_PARAMETER;
+
 	return opal_flash_op(FLASH_OP_READ, id, offset, buf, size, token);
 }
 
 static int64_t opal_flash_write(uint64_t id, uint64_t offset, uint64_t buf,
 		uint64_t size, uint64_t token)
 {
+	if (!opal_addr_valid((void *)buf))
+		return OPAL_PARAMETER;
+
 	return opal_flash_op(FLASH_OP_WRITE, id, offset, buf, size, token);
 }
 
diff --git a/core/i2c.c b/core/i2c.c
index cf9dd67..de74681 100644
--- a/core/i2c.c
+++ b/core/i2c.c
@@ -59,6 +59,9 @@ static int opal_i2c_request(uint64_t async_token, uint32_t bus_id,
 	struct i2c_request *req;
 	int rc;
 
+	if (!opal_addr_valid(oreq))
+		return OPAL_PARAMETER;
+
 	if (oreq->flags & OPAL_I2C_ADDR_10)
 		return OPAL_UNSUPPORTED;
 
diff --git a/core/interrupts.c b/core/interrupts.c
index f93ce7b..5ab853a 100644
--- a/core/interrupts.c
+++ b/core/interrupts.c
@@ -405,6 +405,9 @@ static int64_t opal_get_xive(uint32_t isn, uint16_t *server, uint8_t *priority)
 {
 	struct irq_source *is = irq_find_source(isn);
 
+	if (!opal_addr_valid(server))
+		return OPAL_PARAMETER;
+
 	if (!is || !is->ops->get_xive)
 		return OPAL_PARAMETER;
 
@@ -417,6 +420,9 @@ static int64_t opal_handle_interrupt(uint32_t isn, __be64 *outstanding_event_mas
 	struct irq_source *is = irq_find_source(isn);
 	int64_t rc = OPAL_SUCCESS;
 
+	if (!opal_addr_valid(outstanding_event_mask))
+		return OPAL_PARAMETER;
+
 	/* No source ? return */
 	if (!is || !is->ops->interrupt) {
 		rc = OPAL_PARAMETER;
diff --git a/core/nvram.c b/core/nvram.c
index bde2dce..67661cd 100644
--- a/core/nvram.c
+++ b/core/nvram.c
@@ -30,6 +30,10 @@ static int64_t opal_read_nvram(uint64_t buffer, uint64_t size, uint64_t offset)
 {
 	if (!nvram_ready)
 		return OPAL_HARDWARE;
+
+	if (!opal_addr_valid((void *)buffer))
+		return OPAL_PARAMETER;
+
 	if (offset >= nvram_size || (offset + size) > nvram_size)
 		return OPAL_PARAMETER;
 
@@ -42,6 +46,10 @@ static int64_t opal_write_nvram(uint64_t buffer, uint64_t size, uint64_t offset)
 {
 	if (!nvram_ready)
 		return OPAL_HARDWARE;
+
+	if (!opal_addr_valid((void *)buffer))
+		return OPAL_PARAMETER;
+
 	if (offset >= nvram_size || (offset + size) > nvram_size)
 		return OPAL_PARAMETER;
 	memcpy(nvram_image + offset, (void *)buffer, size);
diff --git a/core/opal-msg.c b/core/opal-msg.c
index 4a7cddb..1971467 100644
--- a/core/opal-msg.c
+++ b/core/opal-msg.c
@@ -81,6 +81,9 @@ static int64_t opal_get_msg(uint64_t *buffer, uint64_t size)
 	if (size < sizeof(struct opal_msg) || !buffer)
 		return OPAL_PARAMETER;
 
+	if (!opal_addr_valid(buffer))
+		return OPAL_PARAMETER;
+
 	lock(&opal_msg_lock);
 
 	entry = list_pop(&msg_pending_list, struct opal_msg_entry, link);
@@ -114,6 +117,9 @@ static int64_t opal_check_completion(uint64_t *buffer, uint64_t size,
 	int rc = OPAL_BUSY;
 	void *data = NULL;
 
+	if (!opal_addr_valid(buffer))
+		return OPAL_PARAMETER;
+
 	lock(&opal_msg_lock);
 	list_for_each_safe(&msg_pending_list, entry, next_entry, link) {
 		if (entry->msg.msg_type == OPAL_MSG_ASYNC_COMP &&
diff --git a/core/opal.c b/core/opal.c
index f48e6ad..986eae2 100644
--- a/core/opal.c
+++ b/core/opal.c
@@ -354,6 +354,10 @@ void opal_run_pollers(void)
 
 static int64_t opal_poll_events(__be64 *outstanding_event_mask)
 {
+
+	if (!opal_addr_valid(outstanding_event_mask))
+		return OPAL_PARAMETER;
+
 	/* Check if we need to trigger an attn for test use */
 	if (attn_trigger == 0xdeadbeef) {
 		prlog(PR_EMERG, "Triggering attn\n");
diff --git a/core/pci-opal.c b/core/pci-opal.c
index ba8e27f..c5a0f71 100644
--- a/core/pci-opal.c
+++ b/core/pci-opal.c
@@ -23,7 +23,27 @@
 #include <timebase.h>
 #include <timer.h>
 
-#define OPAL_PCICFG_ACCESS(op, cb, type)	\
+#define OPAL_PCICFG_ACCESS_READ(op, cb, type)	\
+static int64_t opal_pci_config_##op(uint64_t phb_id,			\
+				    uint64_t bus_dev_func,		\
+				    uint64_t offset, type data)		\
+{									\
+	struct phb *phb = pci_get_phb(phb_id);				\
+	int64_t rc;							\
+									\
+	if (!opal_addr_valid((void *)data))				\
+		return OPAL_PARAMETER;					\
+									\
+	if (!phb)							\
+		return OPAL_PARAMETER;					\
+	phb_lock(phb);							\
+	rc = phb->ops->cfg_##cb(phb, bus_dev_func, offset, data);	\
+	phb_unlock(phb);						\
+									\
+	return rc;							\
+}
+
+#define OPAL_PCICFG_ACCESS_WRITE(op, cb, type)	\
 static int64_t opal_pci_config_##op(uint64_t phb_id,			\
 				    uint64_t bus_dev_func,		\
 				    uint64_t offset, type data)		\
@@ -40,12 +60,12 @@ static int64_t opal_pci_config_##op(uint64_t phb_id,			\
 	return rc;							\
 }
 
-OPAL_PCICFG_ACCESS(read_byte,		read8, uint8_t *)
-OPAL_PCICFG_ACCESS(read_half_word,	read16, uint16_t *)
-OPAL_PCICFG_ACCESS(read_word,		read32, uint32_t *)
-OPAL_PCICFG_ACCESS(write_byte,		write8, uint8_t)
-OPAL_PCICFG_ACCESS(write_half_word,	write16, uint16_t)
-OPAL_PCICFG_ACCESS(write_word,		write32, uint32_t)
+OPAL_PCICFG_ACCESS_READ(read_byte,		read8, uint8_t *)
+OPAL_PCICFG_ACCESS_READ(read_half_word,		read16, uint16_t *)
+OPAL_PCICFG_ACCESS_READ(read_word,		read32, uint32_t *)
+OPAL_PCICFG_ACCESS_WRITE(write_byte,		write8, uint8_t)
+OPAL_PCICFG_ACCESS_WRITE(write_half_word,	write16, uint16_t)
+OPAL_PCICFG_ACCESS_WRITE(write_word,		write32, uint32_t)
 
 opal_call(OPAL_PCI_CONFIG_READ_BYTE, opal_pci_config_read_byte, 4);
 opal_call(OPAL_PCI_CONFIG_READ_HALF_WORD, opal_pci_config_read_half_word, 4);
@@ -82,6 +102,10 @@ static int64_t opal_pci_eeh_freeze_status(uint64_t phb_id, uint64_t pe_number,
 	struct phb *phb = pci_get_phb(phb_id);
 	int64_t rc;
 
+	if (!opal_addr_valid(freeze_state) || !opal_addr_valid(pci_error_type)
+		|| !opal_addr_valid(phb_status))
+		return OPAL_PARAMETER;
+
 	if (!phb)
 		return OPAL_PARAMETER;
 	if (!phb->ops->eeh_freeze_status)
@@ -387,6 +411,9 @@ static int64_t opal_get_xive_source(uint64_t phb_id, uint32_t xive_num,
 	struct phb *phb = pci_get_phb(phb_id);
 	int64_t rc;
 
+	if (!opal_addr_valid(interrupt_source_number))
+		return OPAL_PARAMETER;
+
 	if (!phb)
 		return OPAL_PARAMETER;
 	if (!phb->ops->get_xive_source)
@@ -406,6 +433,9 @@ static int64_t opal_get_msi_32(uint64_t phb_id, uint32_t mve_number,
 	struct phb *phb = pci_get_phb(phb_id);
 	int64_t rc;
 
+	if (!opal_addr_valid(msi_address) || !opal_addr_valid(message_data))
+		return OPAL_PARAMETER;
+
 	if (!phb)
 		return OPAL_PARAMETER;
 	if (!phb->ops->get_msi_32)
@@ -426,6 +456,9 @@ static int64_t opal_get_msi_64(uint64_t phb_id, uint32_t mve_number,
 	struct phb *phb = pci_get_phb(phb_id);
 	int64_t rc;
 
+	if (!opal_addr_valid(msi_address) || !opal_addr_valid(message_data))
+		return OPAL_PARAMETER;
+
 	if (!phb)
 		return OPAL_PARAMETER;
 	if (!phb->ops->get_msi_64)
@@ -632,6 +665,9 @@ static int64_t opal_pci_get_presence_state(uint64_t id, uint64_t data)
 	uint8_t *presence = (uint8_t *)data;
 	int64_t rc;
 
+	if (!opal_addr_valid(presence))
+		return OPAL_PARAMETER;
+
 	if (!slot || !phb)
 		return OPAL_PARAMETER;
 	if (!slot->ops.get_presence_state)
@@ -652,6 +688,9 @@ static int64_t opal_pci_get_power_state(uint64_t id, uint64_t data)
 	uint8_t *power_state = (uint8_t *)data;
 	int64_t rc;
 
+	if (!opal_addr_valid(power_state))
+		return OPAL_PARAMETER;
+
 	if (!slot || !phb)
 		return OPAL_PARAMETER;
 	if (!slot->ops.get_power_state)
@@ -739,6 +778,9 @@ static int64_t opal_pci_set_power_state(uint64_t async_token,
 	if (!slot || !phb)
 		return OPAL_PARAMETER;
 
+	if (!opal_addr_valid(state))
+		return OPAL_PARAMETER;
+
 	phb_lock(phb);
 	switch (*state) {
 	case OPAL_PCI_SLOT_POWER_OFF:
@@ -815,6 +857,9 @@ static int64_t opal_pci_get_phb_diag_data(uint64_t phb_id,
 	struct phb *phb = pci_get_phb(phb_id);
 	int64_t rc;
 
+	if (!opal_addr_valid(diag_buffer))
+		return OPAL_PARAMETER;
+
 	if (!phb)
 		return OPAL_PARAMETER;
 	if (!phb->ops->get_diag_data)
@@ -834,6 +879,9 @@ static int64_t opal_pci_get_phb_diag_data2(uint64_t phb_id,
 	struct phb *phb = pci_get_phb(phb_id);
 	int64_t rc;
 
+	if (!opal_addr_valid(diag_buffer))
+		return OPAL_PARAMETER;
+
 	if (!phb)
 		return OPAL_PARAMETER;
 	if (!phb->ops->get_diag_data2)
@@ -852,6 +900,10 @@ static int64_t opal_pci_next_error(uint64_t phb_id, uint64_t *first_frozen_pe,
 	struct phb *phb = pci_get_phb(phb_id);
 	int64_t rc;
 
+	if (!opal_addr_valid(first_frozen_pe) ||
+		!opal_addr_valid(pci_error_type) || !opal_addr_valid(severity))
+		return OPAL_PARAMETER;
+
 	if (!phb)
 		return OPAL_PARAMETER;
 	if (!phb->ops->next_error)
@@ -876,6 +928,10 @@ static int64_t opal_pci_eeh_freeze_status2(uint64_t phb_id, uint64_t pe_number,
 	struct phb *phb = pci_get_phb(phb_id);
 	int64_t rc;
 
+	if (!opal_addr_valid(freeze_state) || !opal_addr_valid(pci_error_type)
+		|| !opal_addr_valid(severity) || !opal_addr_valid(phb_status))
+		return OPAL_PARAMETER;
+
 	if (!phb)
 		return OPAL_PARAMETER;
 	if (!phb->ops->eeh_freeze_status)
diff --git a/core/test/run-msg.c b/core/test/run-msg.c
index 66b8429..3a7b7dd 100644
--- a/core/test/run-msg.c
+++ b/core/test/run-msg.c
@@ -23,6 +23,9 @@
 static bool zalloc_should_fail = false;
 static int zalloc_should_fail_after = 0;
 
+/* Fake top_of_ram -- needed for API's */
+unsigned long top_of_ram = 16ULL * 1024 * 1024 * 1024;
+
 static void *zalloc(size_t size)
 {
         if (zalloc_should_fail && zalloc_should_fail_after == 0) {
diff --git a/hw/cec.c b/hw/cec.c
index 1743f4d..2f56e81 100644
--- a/hw/cec.c
+++ b/hw/cec.c
@@ -72,6 +72,9 @@ static int64_t opal_pci_get_hub_diag_data(uint64_t hub_id,
 {
 	struct io_hub *hub = cec_get_hub_by_id(hub_id);
 
+	if (!opal_addr_valid(diag_buffer))
+		return OPAL_PARAMETER;
+
 	if (!hub)
 		return OPAL_PARAMETER;
 
-- 
2.5.5



More information about the Skiboot mailing list