From: Jit Loon Lim <jit.loon.lim@intel.com>
To: u-boot@lists.denx.de
Cc: Jagan Teki <jagan@amarulasolutions.com>,
Vignesh R <vigneshr@ti.com>, Marek <marex@denx.de>,
Simon <simon.k.r.goldschmidt@gmail.com>,
Tien Fong <tien.fong.chee@intel.com>,
Kok Kiang <kok.kiang.hea@intel.com>,
Siew Chin <elly.siew.chin.lim@intel.com>,
Sin Hui <sin.hui.kho@intel.com>, Raaj <raaj.lokanathan@intel.com>,
Dinesh <dinesh.maniyam@intel.com>,
Boon Khai <boon.khai.ng@intel.com>,
Alif <alif.zakuan.yuslaimi@intel.com>,
Teik Heng <teik.heng.chong@intel.com>,
Hazim <muhammad.hazim.izzat.zamri@intel.com>,
Jit Loon Lim <jit.loon.lim@intel.com>,
Sieu Mun Tang <sieu.mun.tang@intel.com>
Subject: [PATCH] ddr: altera: n5x: Ensure 'cal->header.data_len' is validated
Date: Wed, 23 Nov 2022 23:21:41 +0800 [thread overview]
Message-ID: <20221123152141.31222-1-jit.loon.lim@intel.com> (raw)
From: Tien Fong Chee <tien.fong.chee@intel.com>
Klocwork reported the unvalidated integer value 'cal->header.data_len' is
used but this is not a issue because the proper value is calculated before
assigning 'cal->header.data_len' and CRC32 is generated before saving
this value into QSPI to ensure data integrity when reading this variable.
Adding checking on 'cal->header.data_len' to ensure the value is valid for
the sake of good coding practice.
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
Signed-off-by: Jit Loon Lim <jit.loon.lim@intel.com>
---
drivers/ddr/altera/sdram_n5x.c | 44 +++++++++++++++++++++++++++-------
1 file changed, 36 insertions(+), 8 deletions(-)
diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
index 0e944b7a15..8a5f0a3df4 100644
--- a/drivers/ddr/altera/sdram_n5x.c
+++ b/drivers/ddr/altera/sdram_n5x.c
@@ -1109,8 +1109,8 @@ static void phy_ocram(phys_addr_t phy_base, phys_addr_t phy_offset,
}
}
-static void cal_data_ocram(phys_addr_t phy_base, u32 addr,
- enum data_process proc)
+static int cal_data_ocram(phys_addr_t phy_base, u32 addr,
+ enum data_process proc)
{
/*
* This array variable contains a list of PHY registers required for
@@ -1435,6 +1435,13 @@ static void cal_data_ocram(phys_addr_t phy_base, u32 addr,
cal->header.ddrconfig_hash,
CHUNKSZ_PER_WD_RESET);
+ if (SOC64_HANDOFF_BASE < ((uintptr_t)(&cal->data) +
+ cal->header.data_len)) {
+ debug("%s: Backup cal data overflow HPS handoff\n",
+ __func__);
+ return -ENOEXEC;
+ }
+
crc32_wd_buf((u8 *)&cal->data, cal->header.data_len,
(u8 *)&cal->header.caldata_crc32,
CHUNKSZ_PER_WD_RESET);
@@ -1443,6 +1450,8 @@ static void cal_data_ocram(phys_addr_t phy_base, u32 addr,
/* Isolate the APB access from internal CSRs */
setbits_le16(phy_base + DDR_PHY_APBONLY0_OFFSET,
DDR_PHY_MICROCONTMUXSEL);
+
+ return 0;
}
static bool is_ddrconfig_hash_match(const void *buffer)
@@ -1559,6 +1568,12 @@ static bool is_cal_bak_data_valid(void)
return false;
}
+ if (SOC64_HANDOFF_BASE < (SOC64_OCRAM_PHY_BACKUP_BASE +
+ cal->header.data_len + sizeof(struct cal_header_t))) {
+ debug("%s: Backup cal data overflow HPS handoff\n", __func__);
+ return false;
+ }
+
/* Load header + DDR bak cal into OCRAM buffer */
ret = request_firmware_into_buf(dev,
qspi_offset,
@@ -1571,6 +1586,12 @@ static bool is_cal_bak_data_valid(void)
return false;
}
+ if (SOC64_HANDOFF_BASE < ((uintptr_t)(&cal->data) +
+ cal->header.data_len)) {
+ debug("%s: Backup cal data overflow HPS handoff\n", __func__);
+ return false;
+ }
+
crc32_wd_buf((u8 *)&cal->data, cal->header.data_len,
(u8 *)&crc32, CHUNKSZ_PER_WD_RESET);
debug("%s: crc32 %x for bak calibration data from QSPI\n", __func__,
@@ -1636,8 +1657,11 @@ static int init_phy(struct ddr_handoff *ddr_handoff_info,
ddr_handoff_info->phy_handoff_length,
ddr_handoff_info->phy_base);
} else {
- cal_data_ocram(ddr_handoff_info->phy_base,
- SOC64_OCRAM_PHY_BACKUP_BASE, LOADING);
+ ret = cal_data_ocram(ddr_handoff_info->phy_base,
+ SOC64_OCRAM_PHY_BACKUP_BASE, LOADING);
+
+ if (ret)
+ return ret;
/*
* Invalidate the section used for processing the PHY
@@ -2955,10 +2979,14 @@ int sdram_mmr_init_full(struct udevice *dev)
* Backup calibration data to OCRAM first, these data
* might be permanant stored to flash in later
*/
- if (is_ddr_retention_enabled(reg))
- cal_data_ocram(ddr_handoff_info.phy_base,
- SOC64_OCRAM_PHY_BACKUP_BASE,
- STORE);
+ if (is_ddr_retention_enabled(reg)) {
+ ret = cal_data_ocram(ddr_handoff_info.phy_base,
+ SOC64_OCRAM_PHY_BACKUP_BASE,
+ STORE);
+
+ if (ret)
+ return ret;
+ }
} else {
/* Updating training result to DDR controller */
--
2.26.2
reply other threads:[~2022-11-23 15:21 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221123152141.31222-1-jit.loon.lim@intel.com \
--to=jit.loon.lim@intel.com \
--cc=alif.zakuan.yuslaimi@intel.com \
--cc=boon.khai.ng@intel.com \
--cc=dinesh.maniyam@intel.com \
--cc=elly.siew.chin.lim@intel.com \
--cc=jagan@amarulasolutions.com \
--cc=kok.kiang.hea@intel.com \
--cc=marex@denx.de \
--cc=muhammad.hazim.izzat.zamri@intel.com \
--cc=raaj.lokanathan@intel.com \
--cc=sieu.mun.tang@intel.com \
--cc=simon.k.r.goldschmidt@gmail.com \
--cc=sin.hui.kho@intel.com \
--cc=teik.heng.chong@intel.com \
--cc=tien.fong.chee@intel.com \
--cc=u-boot@lists.denx.de \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox