From f880eb061bf44101c65487bd74384a1ddcecce5e Mon Sep 17 00:00:00 2001 From: Bill XIE Date: Tue, 10 Nov 2020 15:20:24 +0800 Subject: drivers/ipmi: Handle the condition when (dev->chip_info == NULL) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some former commits (e.g. Ieb41771c75aae902191bba5d220796e6c343f8e0) blindly assume that dev->chip_info is capable to be dereferenced, making at least compilers complain about potential null pointer dereference. They might cause crash if truly (dev->chip_info == NULL). Signed-off-by: Bill XIE Change-Id: I1d694b12f6c42961c104fe839d4ee46c0f111197 Reviewed-on: https://review.coreboot.org/c/coreboot/+/47387 Tested-by: build bot (Jenkins) Reviewed-by: Frans Hendriks Reviewed-by: Michael Niewöhner --- src/drivers/ipmi/ipmi_kcs_ops.c | 56 ++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 29 deletions(-) (limited to 'src/drivers') diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c index 362f17ac2b..640bfa1c17 100644 --- a/src/drivers/ipmi/ipmi_kcs_ops.c +++ b/src/drivers/ipmi/ipmi_kcs_ops.c @@ -76,8 +76,8 @@ static void ipmi_kcs_init(struct device *dev) { struct ipmi_devid_rsp rsp; uint32_t man_id = 0, prod_id = 0; - struct drivers_ipmi_config *conf = NULL; - struct ipmi_selftest_rsp selftestrsp; + struct drivers_ipmi_config *conf = dev->chip_info; + struct ipmi_selftest_rsp selftestrsp = {0}; uint8_t retry_count; if (!dev->enabled) @@ -85,11 +85,13 @@ static void ipmi_kcs_init(struct device *dev) printk(BIOS_DEBUG, "IPMI: PNP KCS 0x%x\n", dev->path.pnp.port); - if (dev->chip_info) - conf = dev->chip_info; + if (!conf) { + printk(BIOS_WARNING, "IPMI: chip_info is missing! Skip init.\n"); + return; + } /* Get IPMI version for ACPI and SMBIOS */ - if (conf && conf->wait_for_bmc && conf->bmc_boot_timeout) { + if (conf->wait_for_bmc && conf->bmc_boot_timeout) { struct stopwatch sw; stopwatch_init_msecs_expire(&sw, conf->bmc_boot_timeout * 1000); printk(BIOS_INFO, "IPMI: Waiting for BMC...\n"); @@ -174,7 +176,7 @@ static unsigned long ipmi_write_acpi_tables(const struct device *dev, unsigned long current, struct acpi_rsdp *rsdp) { - struct drivers_ipmi_config *conf = NULL; + struct drivers_ipmi_config *conf = dev->chip_info; struct acpi_spmi *spmi; s8 gpe_interrupt = -1; u32 apic_interrupt = 0; @@ -204,25 +206,24 @@ ipmi_write_acpi_tables(const struct device *dev, unsigned long current, printk(BIOS_DEBUG, "ACPI: * SPMI at %lx\n", current); spmi = (struct acpi_spmi *)current; - if (dev->chip_info) - conf = dev->chip_info; - if (conf) { if (conf->have_gpe) gpe_interrupt = conf->gpe_interrupt; if (conf->have_apic) apic_interrupt = conf->apic_interrupt; - } - /* Use command to get UID from ipmi_ssdt */ - acpi_create_ipmi(dev, spmi, (ipmi_revision_major << 8) | - (ipmi_revision_minor << 4), &addr, - IPMI_INTERFACE_KCS, gpe_interrupt, apic_interrupt, - conf->uid); + /* Use command to get UID from ipmi_ssdt */ + acpi_create_ipmi(dev, spmi, (ipmi_revision_major << 8) | + (ipmi_revision_minor << 4), &addr, + IPMI_INTERFACE_KCS, gpe_interrupt, apic_interrupt, + conf->uid); - acpi_add_table(rsdp, spmi); + acpi_add_table(rsdp, spmi); - current += spmi->header.length; + current += spmi->header.length; + } else { + printk(BIOS_WARNING, "IPMI: chip_info is missing!\n"); + } return current; } @@ -230,7 +231,7 @@ ipmi_write_acpi_tables(const struct device *dev, unsigned long current, static void ipmi_ssdt(const struct device *dev) { const char *scope = acpi_device_scope(dev); - struct drivers_ipmi_config *conf = NULL; + struct drivers_ipmi_config *conf = dev->chip_info; if (!scope) { printk(BIOS_ERR, "IPMI: Missing ACPI scope for %s\n", @@ -238,8 +239,10 @@ static void ipmi_ssdt(const struct device *dev) return; } - if (dev->chip_info) - conf = dev->chip_info; + if (!conf) { + printk(BIOS_WARNING, "IPMI: chip_info is missing!\n"); + return; + } /* Use command to pass UID to ipmi_write_acpi_tables */ conf->uid = uid_cnt++; @@ -257,11 +260,9 @@ static void ipmi_ssdt(const struct device *dev) acpigen_write_io16(dev->path.pnp.port + CONFIG_IPMI_KCS_REGISTER_SPACING, dev->path.pnp.port + CONFIG_IPMI_KCS_REGISTER_SPACING, 1, 1, 1); - if (conf) { - // FIXME: is that correct? - if (conf->have_apic) - acpigen_write_irq(1 << conf->apic_interrupt); - } + // FIXME: is that correct? + if (conf->have_apic) + acpigen_write_irq(1 << conf->apic_interrupt); acpigen_write_resourcetemplate_footer(); @@ -295,16 +296,13 @@ void ipmi_bmc_version(uint8_t *ipmi_bmc_major_revision, uint8_t *ipmi_bmc_minor_ static int ipmi_smbios_data(struct device *dev, int *handle, unsigned long *current) { - struct drivers_ipmi_config *conf = NULL; + struct drivers_ipmi_config *conf = dev->chip_info; u8 nv_storage = 0xff; u8 i2c_address = 0; u8 register_spacing; int len = 0; - if (dev->chip_info) - conf = dev->chip_info; - if (conf) { if (conf->have_nv_storage) nv_storage = conf->nv_storage_device_address; -- cgit v1.2.3