On 03.08.21 16:28, Pali Rohár wrote:
Variable xyz.len is set to -1 on error. At the end xyzModem_stream_read()
function calls memcpy() with length from variable xyz.len. If this variable
is set to -1 then value passed to memcpy is casted to unsigned value, which
means to copy whole address space. Which then cause U-Boot crash. E.g. on
arm64 it cause CPU crash: "Synchronous Abort" handler, esr 0x96000006
Fix this issue by checking that value stored in xyz.len is valid prior
trying to use it.
Signed-off-by: Pali Rohár <p...@kernel.org>
The changes below seem reasonable.
I would prefer if you could document the elements of struct xyz first.
Just add Sphinx style comments to the structure and indicate for element
len that a negative value signals an error.
Cf.
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation
Acked-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
---
common/xyzModem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/xyzModem.c b/common/xyzModem.c
index fc3459ebbafe..b1b72aae0baf 100644
--- a/common/xyzModem.c
+++ b/common/xyzModem.c
@@ -494,7 +494,7 @@ xyzModem_stream_read (char *buf, int size, int *err)
total = 0;
stat = xyzModem_cancel;
/* Try and get 'size' bytes into the buffer */
- while (!xyz.at_eof && (size > 0))
+ while (!xyz.at_eof && xyz.len >= 0 && (size > 0))
{
if (xyz.len == 0)
{
@@ -587,7 +587,7 @@ xyzModem_stream_read (char *buf, int size, int *err)
}
}
/* Don't "read" data from the EOF protocol package */
- if (!xyz.at_eof)
+ if (!xyz.at_eof && xyz.len > 0)
{
len = xyz.len;
if (size < len)
