From patchwork Mon Jul 18 03:39:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Balbir Singh X-Patchwork-Id: 649313 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3rt88P06jQz9s6r for ; Mon, 18 Jul 2016 13:40:37 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=xC/HHpoO; dkim-atps=neutral Received: from ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3rt88N64zdzDqMF for ; Mon, 18 Jul 2016 13:40:36 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=xC/HHpoO; dkim-atps=neutral X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Received: from mail-pa0-f67.google.com (mail-pa0-f67.google.com [209.85.220.67]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3rt8892PY0zDqDX for ; Mon, 18 Jul 2016 13:40:25 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=xC/HHpoO; dkim-atps=neutral Received: by mail-pa0-f67.google.com with SMTP id ez1so6817575pab.3 for ; Sun, 17 Jul 2016 20:40:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:reply-to:references:mime-version :content-disposition:in-reply-to:user-agent; bh=zYN84nBxWp2QF2crk/oWElyjV5a5LQj2EWc4QNG2QTc=; b=xC/HHpoOYmT2JT8b7wB85vdmoXpwx7LzPS2QBgV2Wn5fzBI/W3QLizJM1UJimA7nf+ 95kEwG40Wkz8bF6mqDOVl6eyeIjsEIaB8H5msGNPxQdAONdnBqQst0EFdQt32ZEI/XGK CQYROjBtQwXiVkEGbJBQT3JzfXMcw4nsCXikllsT8+MYF0xKUPfpy1GnyXUCJ2KzDWS0 EjPRiSMVk/bDu+dUeAwOGLXJvTsxejWN5Y4FkMyF0sCouq1QSbc/GJ33BMIhcAFgR8ad M59F6L7FJgImQUOfcRaxRj6rYg/K0JKOihU1m4JMPEcnTNp/cWc73rJr2oTrwTiVVLMe ctuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:reply-to :references:mime-version:content-disposition:in-reply-to:user-agent; bh=zYN84nBxWp2QF2crk/oWElyjV5a5LQj2EWc4QNG2QTc=; b=erwyVSzIRvcoEWrePVzpzOT2bB4iMANEgb5EfslPs/jdOROt+bxADECBCwVsl3uWAm WMW49mBG1NBgOFqJEn9t3wabLM3TOk8uSHgL6Ol3a8OCeVcsfjJPlQ5lX1b3KWS5vQDO DMUcLgBl3Zfq1nNPL6udJIxBLdMOhHHWbuB3FCF0oakwABgEe3ZQWGKRs+WtndcZtJxE vgUfK5hy8c325IAwdxLCLN4JvJ4+Zlb4eqaP1guBGrxK2ECdG37+nwB/cc/bIykgiM/K fRji2UCaetzg4iOMsxH+VFkVzoqWC6WL2Tgf6TdxAFfLjdaGk3L9qOLbQ9XueAIg2E26 ucwA== X-Gm-Message-State: ALyK8tKy9yLpWczhm6S/u2cQFG7atyUS7lbggtGiqEr0kHM2I5zJ44SjdJUOP1Yqoxe1EA== X-Received: by 10.66.139.133 with SMTP id qy5mr52918445pab.4.1468813163113; Sun, 17 Jul 2016 20:39:23 -0700 (PDT) Received: from balbir.ozlabs.ibm.com ([122.99.82.10]) by smtp.gmail.com with ESMTPSA id k78sm577679pfa.78.2016.07.17.20.39.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 17 Jul 2016 20:39:22 -0700 (PDT) Date: Mon, 18 Jul 2016 13:39:16 +1000 From: Balbir Singh To: Balbir Singh Message-ID: <20160718033916.GD5809@balbir.ozlabs.ibm.com> References: <20160718033525.GB5809@balbir.ozlabs.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160718033525.GB5809@balbir.ozlabs.ibm.com> User-Agent: Mutt/1.6.1 (2016-04-27) Subject: [Skiboot] [PATCH 2/2] Use additional checks in skiboot for pointers X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: bsingharora@gmail.com Cc: skiboot@lists.ozlabs.org Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" 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 --- 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 #include -#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;