VBE is currently quite careful with function arguments because it is used in VPL which cannot be updated after manufacture. Bugs can cause security holes.
Unfortunately this adds to code size.
In several cases we are reading values from a devicetree which is part of U-Boot (or at least VPL) and so known to be good. Also, in several places, getting bad values does not matter.
So change a few checks to assert() to reduce code size.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add new patch to convert some checks to assertions
boot/vbe_simple.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/boot/vbe_simple.c b/boot/vbe_simple.c index b6f0a49c584..af16bdd34a3 100644 --- a/boot/vbe_simple.c +++ b/boot/vbe_simple.c @@ -27,12 +27,17 @@ static int simple_read_version(const struct simple_priv *priv, { int start;
- if (priv->version_size > MMC_MAX_BLOCK_LEN) - return log_msg_ret("ver", -E2BIG); + /* we can use an assert() here since we already read only one block */ + assert(priv->version_size <= MMC_MAX_BLOCK_LEN);
start = priv->area_start + priv->version_offset; - if (start & (MMC_MAX_BLOCK_LEN - 1)) - return log_msg_ret("get", -EBADF); + + /* + * we can use an assert() here since reading the wrong block will just + * cause an invalid version-string to be (safely) read + */ + assert(!(start & (MMC_MAX_BLOCK_LEN - 1))); + start /= MMC_MAX_BLOCK_LEN;
if (blk_read(blk, start, 1, buf) != 1) @@ -51,12 +56,21 @@ static int simple_read_nvdata(const struct simple_priv *priv, const struct vbe_nvdata *nvd; int start;
- if (priv->state_size > MMC_MAX_BLOCK_LEN) - return log_msg_ret("state", -E2BIG); + /* we can use an assert() here since we already read only one block */ + assert(priv->state_size <= MMC_MAX_BLOCK_LEN);
start = priv->area_start + priv->state_offset; - if (start & (MMC_MAX_BLOCK_LEN - 1)) - return log_msg_ret("get", -EBADF); + + /* + * We can use an assert() here since reading the wrong block will just + * cause invalid state to be (safely) read. If the crc passes, then we + * obtain invalid state and it will likely cause booting to fail. + * + * VBE relies on valid values being in U-Boot's devicetree, so this + * should not every be wrong on a production device. + */ + assert(!(start & (MMC_MAX_BLOCK_LEN - 1))); + start /= MMC_MAX_BLOCK_LEN;
if (blk_read(blk, start, 1, buf) != 1) @@ -67,7 +81,7 @@ static int simple_read_nvdata(const struct simple_priv *priv, if (hdr_ver != NVD_HDR_VER_CUR) return log_msg_ret("hdr", -EPERM); size = 1 << hdr_size; - if (size > sizeof(*nvd)) + if (!size || size > sizeof(*nvd)) return log_msg_ret("sz", -ENOEXEC);
crc = crc8(0, buf + 1, size - 1);