From 767c4b28998aff563c36e2f2000e4768b68add8f Mon Sep 17 00:00:00 2001 From: Jacob Garber Date: Mon, 22 Jul 2019 13:31:38 -0600 Subject: soc/intel/baytrail: Prevent unintended sign extensions Consider the following assignment: u64 = s32 For positive values this is fine, but if the s32 is negative, it will be sign-extended in the conversion to a very large unsigned integer. This manifests itself in two ways in the following code: First, gpu_pipe{a,b}_port_select are defined as int, and can have the values 1 or 2. In the case when they have the value 2, the shift 2 << 30 will be a negative number, making it susceptible to the sign-extension problem above. Change these variables to something more reasonable like a uint8_t, which is unsigned. Second, in any bit shift, any variable with width less than an int will be implicitly promoted to an int before performing the bit shift. For example, the variable gpu_pipea_power_on_delay is a uint16_t, and if its highest bit is set, the shift gpu_pipea_power_on_delay << 16 will become negative, again introducing the above problem. To prevent this, cast all smaller variables to a u32 before the shift, which will prevent the implicit promotions and sign extensions. Change-Id: Ic5db6001504cefb501dee199590a0e961a15771b Signed-off-by: Jacob Garber Found-by: Coverity CID 1229699, 1229700, 1229701, 1229702 Reviewed-on: https://review.coreboot.org/c/coreboot/+/34487 Reviewed-by: Angel Pons Reviewed-by: Alexander Couzens Tested-by: build bot (Jenkins) --- src/soc/intel/baytrail/chip.h | 4 ++-- src/soc/intel/baytrail/gfx.c | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/soc/intel/baytrail/chip.h b/src/soc/intel/baytrail/chip.h index 7736e721af..00e7fd6f73 100644 --- a/src/soc/intel/baytrail/chip.h +++ b/src/soc/intel/baytrail/chip.h @@ -69,7 +69,7 @@ struct soc_intel_baytrail_config { /* Allow PCIe devices to wake system from suspend. */ int pcie_wake_enable; - int gpu_pipea_port_select; /* Port select: 1=DP_B 2=DP_C */ + uint8_t gpu_pipea_port_select; /* Port select: 1=DP_B 2=DP_C */ uint16_t gpu_pipea_power_on_delay; uint16_t gpu_pipea_light_on_delay; uint16_t gpu_pipea_power_off_delay; @@ -77,7 +77,7 @@ struct soc_intel_baytrail_config { uint16_t gpu_pipea_power_cycle_delay; int gpu_pipea_pwm_freq_hz; - int gpu_pipeb_port_select; /* Port select: 1=DP_B 2=DP_C */ + uint8_t gpu_pipeb_port_select; /* Port select: 1=DP_B 2=DP_C */ uint16_t gpu_pipeb_power_on_delay; uint16_t gpu_pipeb_light_on_delay; uint16_t gpu_pipeb_power_off_delay; diff --git a/src/soc/intel/baytrail/gfx.c b/src/soc/intel/baytrail/gfx.c index 4a799916db..5100c8e777 100644 --- a/src/soc/intel/baytrail/gfx.c +++ b/src/soc/intel/baytrail/gfx.c @@ -320,13 +320,13 @@ static void gfx_panel_setup(struct device *dev) PP_CONTROL_UNLOCK | PP_CONTROL_EDP_FORCE_VDD), /* POWER ON */ REG_RES_WRITE32(PCI_BASE_ADDRESS_0, PIPEA_REG(PP_ON_DELAYS), - (config->gpu_pipea_port_select << 30 | - config->gpu_pipea_power_on_delay << 16 | - config->gpu_pipea_light_on_delay)), + ((u32)config->gpu_pipea_port_select << 30 | + (u32)config->gpu_pipea_power_on_delay << 16 | + (u32)config->gpu_pipea_light_on_delay)), /* POWER OFF */ REG_RES_WRITE32(PCI_BASE_ADDRESS_0, PIPEA_REG(PP_OFF_DELAYS), - (config->gpu_pipea_power_off_delay << 16 | - config->gpu_pipea_light_off_delay)), + ((u32)config->gpu_pipea_power_off_delay << 16 | + (u32)config->gpu_pipea_light_off_delay)), /* DIVISOR */ REG_RES_RMW32(PCI_BASE_ADDRESS_0, PIPEA_REG(PP_DIVISOR), ~0x1f, config->gpu_pipea_power_cycle_delay), @@ -338,13 +338,13 @@ static void gfx_panel_setup(struct device *dev) PP_CONTROL_UNLOCK | PP_CONTROL_EDP_FORCE_VDD), /* POWER ON */ REG_RES_WRITE32(PCI_BASE_ADDRESS_0, PIPEB_REG(PP_ON_DELAYS), - (config->gpu_pipeb_port_select << 30 | - config->gpu_pipeb_power_on_delay << 16 | - config->gpu_pipeb_light_on_delay)), + ((u32)config->gpu_pipeb_port_select << 30 | + (u32)config->gpu_pipeb_power_on_delay << 16 | + (u32)config->gpu_pipeb_light_on_delay)), /* POWER OFF */ REG_RES_WRITE32(PCI_BASE_ADDRESS_0, PIPEB_REG(PP_OFF_DELAYS), - (config->gpu_pipeb_power_off_delay << 16 | - config->gpu_pipeb_light_off_delay)), + ((u32)config->gpu_pipeb_power_off_delay << 16 | + (u32)config->gpu_pipeb_light_off_delay)), /* DIVISOR */ REG_RES_RMW32(PCI_BASE_ADDRESS_0, PIPEB_REG(PP_DIVISOR), ~0x1f, config->gpu_pipeb_power_cycle_delay), -- cgit v1.2.3