Anastasia Klimchuk submitted this change.

View Change

erasure_layout: don't copy region buffers if they're null/zero-size

memcpy() function expects 2nd parameter to be non-null. Make sure that
the pointer is non-null before passing it to the function.

Also move allocations under if conditions to avoid allocating memory for
a potentially 0 size.

Found-by: scan-build, clang v17.0.6
Signed-off-by: Alexander Goncharov <chat@joursoir.net>
Change-Id: I99aad4119f9c11bea75e3419926cc833bc1f68c5
Reviewed-on: https://review.coreboot.org/c/flashrom/+/81548
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M erasure_layout.c
1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/erasure_layout.c b/erasure_layout.c
index 1dd6b60..e9bfa86 100644
--- a/erasure_layout.c
+++ b/erasure_layout.c
@@ -260,29 +260,31 @@
 	align_region(erase_layout, flashctx, &region_start, &region_end);
 
 	uint8_t *old_start_buf = NULL, *old_end_buf = NULL;
-	old_start_buf = (uint8_t *)malloc(old_start - region_start);
-	if (!old_start_buf) {
-		msg_cerr("Not enough memory!\n");
-		ret = -1;
-		goto _end;
-	}
-	old_end_buf = (uint8_t *)malloc(region_end - old_end);
-	if (!old_end_buf) {
-		msg_cerr("Not enough memory!\n");
-		ret = -1;
-		goto _end;
-	}
+	const size_t start_buf_len = old_start - region_start;
+	const size_t end_buf_len = region_end - old_end;
 
-	if (old_start - region_start) {
-		read_flash(flashctx, curcontents + region_start, region_start, old_start - region_start);
-		memcpy(old_start_buf, newcontents + region_start, old_start - region_start);
-		memcpy(newcontents + region_start, curcontents + region_start, old_start - region_start);
+	if (start_buf_len) {
+		old_start_buf = (uint8_t *)malloc(start_buf_len);
+		if (!old_start_buf) {
+			msg_cerr("Not enough memory!\n");
+			ret = -1;
+			goto _end;
+		}
+		read_flash(flashctx, curcontents + region_start, region_start, start_buf_len);
+		memcpy(old_start_buf, newcontents + region_start, start_buf_len);
+		memcpy(newcontents + region_start, curcontents + region_start, start_buf_len);
 	}
-	if (region_end - old_end) {
+	if (end_buf_len) {
 		chipoff_t end_offset = old_end + 1;
-		read_flash(flashctx, curcontents + end_offset, end_offset, region_end - old_end);
-		memcpy(old_end_buf, newcontents + end_offset, region_end - old_end);
-		memcpy(newcontents + end_offset, curcontents + end_offset, region_end - old_end);
+		old_end_buf = (uint8_t *)malloc(end_buf_len);
+		if (!old_end_buf) {
+			msg_cerr("Not enough memory!\n");
+			ret = -1;
+			goto _end;
+		}
+		read_flash(flashctx, curcontents + end_offset, end_offset, end_buf_len);
+		memcpy(old_end_buf, newcontents + end_offset, end_buf_len);
+		memcpy(newcontents + end_offset, curcontents + end_offset, end_buf_len);
 	}
 
 	// select erase functions
@@ -381,11 +383,15 @@
 	}
 
 _end:
-	memcpy(newcontents + region_start, old_start_buf, old_start - region_start);
-	memcpy(newcontents + old_end, old_end_buf, region_end - old_end);
+	if (old_start_buf) {
+		memcpy(newcontents + region_start, old_start_buf, start_buf_len);
+		free(old_start_buf);
+	}
 
-	free(old_start_buf);
-	free(old_end_buf);
+	if (old_end_buf) {
+		memcpy(newcontents + old_end, old_end_buf, end_buf_len);
+		free(old_end_buf);
+	}
 
 	msg_cinfo("Erase/write done from %"PRIx32" to %"PRIx32"\n", region_start, region_end);
 	return ret;

To view, visit change 81548. To unsubscribe, or for help writing mail filters, visit settings.