This is a theoretical possible out of bounds error in DDR driver. Adding check before using array index. Also change some runtime conditions to pre-compiling conditions.
Signed-off-by: York Sun yorksun@freescale.com --- Change log: v2: Revise subject and commit message
drivers/ddr/fsl/ctrl_regs.c | 8 ++-- drivers/ddr/fsl/main.c | 6 +++ drivers/ddr/fsl/options.c | 108 +++++++++++++++++++++---------------------- 3 files changed, 64 insertions(+), 58 deletions(-)
diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c index d01eea0..78e82bb 100644 --- a/drivers/ddr/fsl/ctrl_regs.c +++ b/drivers/ddr/fsl/ctrl_regs.c @@ -507,8 +507,8 @@ static void set_timing_cfg_1(fsl_ddr_cfg_regs_t *ddr, wrrec_mclk = picos_to_mclk(common_dimm->twr_ps); acttoact_mclk = max(picos_to_mclk(common_dimm->trrds_ps), 4); wrtord_mclk = max(2, picos_to_mclk(2500)); - if (wrrec_mclk > 24) - printf("Error: WRREC doesn't support more than 24 clocks\n"); + if ((wrrec_mclk < 1) || (wrrec_mclk > 24)) + printf("Error: WRREC doesn't support %d clocks\n", wrrec_mclk); else wrrec_mclk = wrrec_table[wrrec_mclk - 1]; #else @@ -516,8 +516,8 @@ static void set_timing_cfg_1(fsl_ddr_cfg_regs_t *ddr, wrrec_mclk = picos_to_mclk(common_dimm->twr_ps); acttoact_mclk = picos_to_mclk(common_dimm->trrd_ps); wrtord_mclk = picos_to_mclk(common_dimm->twtr_ps); - if (wrrec_mclk > 16) - printf("Error: WRREC doesn't support more than 16 clocks\n"); + if ((wrrec_mclk < 1) || (wrrec_mclk > 16)) + printf("Error: WRREC doesn't support %d clocks\n", wrrec_mclk); else wrrec_mclk = wrrec_table[wrrec_mclk - 1]; #endif diff --git a/drivers/ddr/fsl/main.c b/drivers/ddr/fsl/main.c index f95704f..5e001fc 100644 --- a/drivers/ddr/fsl/main.c +++ b/drivers/ddr/fsl/main.c @@ -220,6 +220,11 @@ const char * step_to_string(unsigned int step) { if ((1 << s) != step) return step_string_tbl[7];
+ if (s >= ARRAY_SIZE(step_string_tbl)) { + printf("Error for the step in %s\n", __func__); + s = 0; + } + return step_string_tbl[s]; }
@@ -520,6 +525,7 @@ fsl_ddr_compute(fsl_ddr_info_t *pinfo, unsigned int start_step, /* STEP 5: Assign addresses to chip selects */ check_interleaving_options(pinfo); total_mem = step_assign_addresses(pinfo, dbw_capacity_adjust); + debug("Total mem %llu assigned\n", total_mem);
case STEP_COMPUTE_REGS: /* STEP 6: compute controller register values */ diff --git a/drivers/ddr/fsl/options.c b/drivers/ddr/fsl/options.c index bf0d1da..5986e1a 100644 --- a/drivers/ddr/fsl/options.c +++ b/drivers/ddr/fsl/options.c @@ -525,67 +525,66 @@ unsigned int populate_memctl_options(int all_dimms_registered, defined(CONFIG_SYS_FSL_DDR2) || \ defined(CONFIG_SYS_FSL_DDR4) /* Chip select options. */ - if (CONFIG_DIMM_SLOTS_PER_CTLR == 1) { - switch (pdimm[0].n_ranks) { - case 1: - pdodt = single_S; - break; +#if (CONFIG_DIMM_SLOTS_PER_CTLR == 1) + switch (pdimm[0].n_ranks) { + case 1: + pdodt = single_S; + break; + case 2: + pdodt = single_D; + break; + case 4: + pdodt = single_Q; + break; + } +#elif (CONFIG_DIMM_SLOTS_PER_CTLR == 2) + switch (pdimm[0].n_ranks) { +#ifdef CONFIG_FSL_DDR_FIRST_SLOT_QUAD_CAPABLE + case 4: + pdodt = single_Q; + if (pdimm[1].n_ranks) + printf("Error: Quad- and Dual-rank DIMMs cannot be used together\n"); + break; +#endif + case 2: + switch (pdimm[1].n_ranks) { case 2: - pdodt = single_D; + pdodt = dual_DD; break; - case 4: - pdodt = single_Q; + case 1: + pdodt = dual_DS; break; - } - } else if (CONFIG_DIMM_SLOTS_PER_CTLR == 2) { - switch (pdimm[0].n_ranks) { -#ifdef CONFIG_FSL_DDR_FIRST_SLOT_QUAD_CAPABLE - case 4: - pdodt = single_Q; - if (pdimm[1].n_ranks) - printf("Error: Quad- and Dual-rank DIMMs " - "cannot be used together\n"); + case 0: + pdodt = dual_D0; break; -#endif + } + break; + case 1: + switch (pdimm[1].n_ranks) { case 2: - switch (pdimm[1].n_ranks) { - case 2: - pdodt = dual_DD; - break; - case 1: - pdodt = dual_DS; - break; - case 0: - pdodt = dual_D0; - break; - } + pdodt = dual_SD; break; case 1: - switch (pdimm[1].n_ranks) { - case 2: - pdodt = dual_SD; - break; - case 1: - pdodt = dual_SS; - break; - case 0: - pdodt = dual_S0; - break; - } + pdodt = dual_SS; break; case 0: - switch (pdimm[1].n_ranks) { - case 2: - pdodt = dual_0D; - break; - case 1: - pdodt = dual_0S; - break; - } + pdodt = dual_S0; break; } + break; + case 0: + switch (pdimm[1].n_ranks) { + case 2: + pdodt = dual_0D; + break; + case 1: + pdodt = dual_0S; + break; + } + break; } -#endif +#endif /* CONFIG_DIMM_SLOTS_PER_CTLR */ +#endif /* CONFIG_SYS_FSL_DDR2, 3, 4 */
/* Pick chip-select local options. */ for (i = 0; i < CONFIG_CHIP_SELECTS_PER_CTRL; i++) { @@ -847,8 +846,7 @@ unsigned int populate_memctl_options(int all_dimms_registered, popts->memctl_interleaving_mode = FSL_DDR_256B_INTERLEAVING; popts->memctl_interleaving = 1; debug("256 Byte interleaving\n"); - goto done; -#endif +#else /* * test null first. if CONFIG_HWCONFIG is not defined * hwconfig_arg_cmp returns non-zero @@ -930,8 +928,9 @@ unsigned int populate_memctl_options(int all_dimms_registered, popts->memctl_interleaving = 0; printf("hwconfig has unrecognized parameter for ctlr_intlv.\n"); } +#endif /* CONFIG_SYS_FSL_DDR_INTLV_256B */ done: -#endif +#endif /* CONFIG_NUM_DDR_CONTROLLERS > 1 */ if ((hwconfig_sub_f("fsl_ddr", "bank_intlv", buf)) && (CONFIG_CHIP_SELECTS_PER_CTRL > 1)) { /* test null first. if CONFIG_HWCONFIG is not defined, @@ -1106,10 +1105,11 @@ void check_interleaving_options(fsl_ddr_info_t *pinfo) case FSL_DDR_PAGE_INTERLEAVING: case FSL_DDR_BANK_INTERLEAVING: case FSL_DDR_SUPERBANK_INTERLEAVING: - if (3 == CONFIG_NUM_DDR_CONTROLLERS) +#if (3 == CONFIG_NUM_DDR_CONTROLLERS) k = 2; - else +#else k = CONFIG_NUM_DDR_CONTROLLERS; +#endif break; case FSL_DDR_3WAY_1KB_INTERLEAVING: case FSL_DDR_3WAY_4KB_INTERLEAVING: