diff options
author | Jacob Garber <jgarber1@ualberta.ca> | 2019-06-26 16:18:16 -0600 |
---|---|---|
committer | Patrick Georgi <pgeorgi@google.com> | 2019-08-20 15:27:42 +0000 |
commit | 9172b6920cac2c4dabf19e529dbfed91b15685c5 (patch) | |
tree | 761b138ce45fface88f8babba31d48bff43203d5 /src/northbridge | |
parent | 5fa756cc97de1ed30ac3fd4d5ddb85f079efe521 (diff) | |
download | coreboot-9172b6920cac2c4dabf19e529dbfed91b15685c5.tar.xz |
src: Remove variable length arrays
Variable length arrays were a feature added in C99 that allows the
length of an array to be determined at runtime. Eg.
int sum(size_t n) {
int arr[n];
...
}
This adds a small amount of runtime overhead, but is also very
dangerous, since it allows use of an unlimited amount of stack memory,
potentially leading to stack overflow. This is only worsened in
coreboot, which often has very little stack space to begin with. Citing
concerns like this, all instances of VLA's were recently removed from the
Linux kernel. In the immortal words of Linus Torvalds [0],
AND USING VLA'S IS ACTIVELY STUPID! It generates much more code, and
much _slower_ code (and more fragile code), than just using a fixed
key size would have done. [...] Anyway, some of these are definitely
easy to just fix, and using VLA's is actively bad not just for
security worries, but simply because VLA's are a really horribly bad
idea in general in the kernel.
This patch follows suit and zaps all VLA's in coreboot. Some of the
existing VLA's are accidental ones, and all but one can be replaced with
small fixed-size buffers. The single tricky exception is in the SPI
controller interface, which will require a rewrite of old drivers
to remove [1].
[0] https://lkml.org/lkml/2018/3/7/621
[1] https://ticket.coreboot.org/issues/217
Change-Id: I7d9d1ddadbf1cee5f695165bbe3f0effb7bd32b9
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33821
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Diffstat (limited to 'src/northbridge')
-rw-r--r-- | src/northbridge/amd/amdmct/mct_ddr3/mctdqs_d.c | 20 | ||||
-rw-r--r-- | src/northbridge/amd/amdmct/mct_ddr3/mhwlc_d.c | 17 |
2 files changed, 29 insertions, 8 deletions
diff --git a/src/northbridge/amd/amdmct/mct_ddr3/mctdqs_d.c b/src/northbridge/amd/amdmct/mct_ddr3/mctdqs_d.c index 3f4f1bf01d..d34b2dc2ba 100644 --- a/src/northbridge/amd/amdmct/mct_ddr3/mctdqs_d.c +++ b/src/northbridge/amd/amdmct/mct_ddr3/mctdqs_d.c @@ -1252,11 +1252,15 @@ void write_dram_dqs_training_pattern_fam15(struct MCTStatStruc *pMCTstat, stop_dram_dqs_training_pattern_fam15(pMCTstat, pDCTstat, dct, Receiver); } +#define LANE_DIFF 1 + /* DQS Position Training * Algorithm detailed in the Fam15h BKDG Rev. 3.14 section 2.10.5.8.4 */ static uint8_t TrainDQSRdWrPos_D_Fam15(struct MCTStatStruc *pMCTstat, - struct DCTStatStruc *pDCTstat, uint8_t dct, uint8_t receiver_start, uint8_t receiver_end, uint8_t lane_start, uint8_t lane_end) + struct DCTStatStruc *pDCTstat, + uint8_t dct, uint8_t receiver_start, + uint8_t receiver_end, uint8_t lane_start) { uint8_t dimm; uint8_t lane; @@ -1276,7 +1280,8 @@ static uint8_t TrainDQSRdWrPos_D_Fam15(struct MCTStatStruc *pMCTstat, uint16_t current_read_dqs_delay[MAX_BYTE_LANES]; uint16_t current_write_dqs_delay[MAX_BYTE_LANES]; uint8_t passing_dqs_delay_found[MAX_BYTE_LANES]; - uint8_t dqs_results_array[2][(lane_end - lane_start)][32][48]; /* [rank][lane][write step][read step + 16] */ + /* [rank][lane][write step][read step + 16] */ + uint8_t dqs_results_array[2][LANE_DIFF][32][48]; uint8_t last_pos = 0; uint8_t cur_count = 0; @@ -1286,6 +1291,8 @@ static uint8_t TrainDQSRdWrPos_D_Fam15(struct MCTStatStruc *pMCTstat, uint32_t index_reg = 0x98; uint32_t dev = pDCTstat->dev_dct; + uint8_t lane_end = lane_start + LANE_DIFF; + uint8_t lane_count; lane_count = get_available_lane_count(pMCTstat, pDCTstat); @@ -1734,7 +1741,10 @@ static void TrainDQSReceiverEnCyc_D_Fam15(struct MCTStatStruc *pMCTstat, Calc_SetMaxRdLatency_D_Fam15(pMCTstat, pDCTstat, dct, 0); /* 2.10.5.8.3 (4 B) */ - dqs_results_array[current_phy_phase_delay[lane]] = TrainDQSRdWrPos_D_Fam15(pMCTstat, pDCTstat, dct, Receiver, Receiver + 2, lane, lane + 1); + dqs_results_array[current_phy_phase_delay[lane]] = + TrainDQSRdWrPos_D_Fam15(pMCTstat, pDCTstat, dct, + Receiver, Receiver + 2, + lane); if (dqs_results_array[current_phy_phase_delay[lane]]) lane_success_count++; @@ -1790,7 +1800,9 @@ static void TrainDQSReceiverEnCyc_D_Fam15(struct MCTStatStruc *pMCTstat, /* Update hardware registers with final values */ write_dqs_receiver_enable_control_registers(current_phy_phase_delay, dev, dct, dimm, index_reg); - TrainDQSRdWrPos_D_Fam15(pMCTstat, pDCTstat, dct, Receiver, Receiver + 2, lane, lane + 1); + TrainDQSRdWrPos_D_Fam15(pMCTstat, pDCTstat, dct, + Receiver, Receiver + 2, + lane); break; } prev = dqs_results_array[current_phy_phase_delay[lane]]; diff --git a/src/northbridge/amd/amdmct/mct_ddr3/mhwlc_d.c b/src/northbridge/amd/amdmct/mct_ddr3/mhwlc_d.c index 76f72ae917..353aa7a1cf 100644 --- a/src/northbridge/amd/amdmct/mct_ddr3/mhwlc_d.c +++ b/src/northbridge/amd/amdmct/mct_ddr3/mhwlc_d.c @@ -15,6 +15,7 @@ */ #include <stdint.h> +#include <assert.h> #include <console/console.h> #include <northbridge/amd/amdfam10/amdfam10.h> @@ -31,6 +32,8 @@ void procConfig(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstat, ui void setWLByteDelay(struct DCTStatStruc *pDCTstat, uint8_t dct, u8 ByteLane, u8 dimm, u8 targetAddr, uint8_t pass, uint8_t lane_count); void getWLByteDelay(struct DCTStatStruc *pDCTstat, uint8_t dct, u8 ByteLane, u8 dimm, uint8_t pass, uint8_t nibble, uint8_t lane_count); +#define MAX_LANE_COUNT 9 + /*----------------------------------------------------------------------------- * uint8_t AgesaHwWlPhase1(SPDStruct *SPDData,MCTStruct *MCTData, DCTStruct *DCTData, * u8 Dimm, u8 Pass) @@ -185,8 +188,10 @@ uint8_t AgesaHwWlPhase2(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCT lane_count = get_available_lane_count(pMCTstat, pDCTstat); + assert(lane_count <= MAX_LANE_COUNT); + if (is_fam15h()) { - int32_t gross_diff[lane_count]; + int32_t gross_diff[MAX_LANE_COUNT]; int32_t cgd = pDCTData->WLCriticalGrossDelayPrevPass; uint8_t index = (uint8_t)(lane_count * dimm); @@ -274,9 +279,11 @@ uint8_t AgesaHwWlPhase3(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCT lane_count = get_available_lane_count(pMCTstat, pDCTstat); + assert(lane_count <= MAX_LANE_COUNT); + if (is_fam15h()) { uint32_t dword; - int32_t gross_diff[lane_count]; + int32_t gross_diff[MAX_LANE_COUNT]; int32_t cgd = pDCTData->WLCriticalGrossDelayPrevPass; uint8_t index = (uint8_t)(lane_count * dimm); @@ -1005,6 +1012,8 @@ void procConfig(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstat, ui lane_count = get_available_lane_count(pMCTstat, pDCTstat); + assert(lane_count <= MAX_LANE_COUNT); + if (is_fam15h()) { /* MemClkFreq: 0x4: 333MHz; 0x6: 400MHz; 0xa: 533MHz; 0xe: 667MHz; 0x12: 800MHz; 0x16: 933MHz */ MemClkFreq = get_Bits(pDCTData, dct, pDCTData->NodeId, @@ -1168,8 +1177,8 @@ void procConfig(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstat, ui /* From BKDG, Write Leveling Seed Value. */ if (is_fam15h()) { uint32_t RegisterDelay; - int32_t SeedTotal[lane_count]; - int32_t SeedTotalPreScaling[lane_count]; + int32_t SeedTotal[MAX_LANE_COUNT]; + int32_t SeedTotalPreScaling[MAX_LANE_COUNT]; uint32_t WrDqDqsEarly; uint8_t AddrCmdPrelaunch = 0; /* TODO: Fetch the correct value from RC2[0] */ |