drivers/ddr: Fix possible out of bounds error

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>
Reviewed-by: York Sun <yorksun@freescale.com>

diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
index d01eea05f371d6fa7f342e89b0db0d198443dcae..78e82bba3d4302a3f34efe1ea0df8a70b0c406fc 100644 (file)
--- 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 f95704f3dfd0fb1eab6d90fc2511f2e5e5952c80..5e001fcb99474605d0c5fadde287e1ef257184b5 100644 (file)
--- 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 bf0d1da7cfa44688b362b1fbf9c117c710d24aaa..5986e1a0b9da3b5099fcdac4b46594c9b1fc3180 100644 (file)
--- 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: