diff mbox series

[1/2] core/flash: NULL pointer dereference fixes

Message ID 20181017144533.12571-1-npiggin@gmail.com
State Accepted
Headers show
Series [1/2] core/flash: NULL pointer dereference fixes | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success master/apply_patch Successfully applied

Commit Message

Nicholas Piggin Oct. 17, 2018, 2:45 p.m. UTC 
These were caught with unmapped memory dereference page faults.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 core/flash-subpartition.c | 15 +++++++++------
 core/flash.c              |  6 +++++-
 2 files changed, 14 insertions(+), 7 deletions(-)

Comments

Stewart Smith Oct. 25, 2018, 11:08 p.m. UTC | #1 
Nicholas Piggin <npiggin@gmail.com> writes:
> These were caught with unmapped memory dereference page faults.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  core/flash-subpartition.c | 15 +++++++++------
>  core/flash.c              |  6 +++++-
>  2 files changed, 14 insertions(+), 7 deletions(-)

Thanks, series merged to master as of
c4230046ecd5b334b1a3a9d8145f8264b1d4f981

Did we see any real failures with this, as in, should we backport to stable?
Nicholas Piggin Oct. 30, 2018, 4:19 a.m. UTC | #2 
On Fri, 26 Oct 2018 10:08:30 +1100
Stewart Smith <stewart@linux.ibm.com> wrote:

> Nicholas Piggin <npiggin@gmail.com> writes:
> > These were caught with unmapped memory dereference page faults.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  core/flash-subpartition.c | 15 +++++++++------
> >  core/flash.c              |  6 +++++-
> >  2 files changed, 14 insertions(+), 7 deletions(-)  
> 
> Thanks, series merged to master as of
> c4230046ecd5b334b1a3a9d8145f8264b1d4f981
> 
> Did we see any real failures with this, as in, should we backport to stable?

No failures in upstream skiboot. I think by luck, *(uint32_t *)0 == 0

The NULL branch catcher writes a function address there, but the first
4 bytes of it are zeroes.

Thanks,
Nick
diff mbox series

Patch

diff --git a/core/flash-subpartition.c b/core/flash-subpartition.c
index 641448d5..7931790c 100644
--- a/core/flash-subpartition.c
+++ b/core/flash-subpartition.c
@@ -32,17 +32,18 @@  struct flash_hostboot_header {
 };
 
 int flash_subpart_info(void *part_header, uint32_t header_len,
-		       uint32_t part_size, uint32_t *part_actual,
+		       uint32_t part_size, uint32_t *part_actualp,
 		       uint32_t subid, uint32_t *offset, uint32_t *size)
 {
 	struct flash_hostboot_header *header;
 	char eyecatcher[5];
 	uint32_t i, ec, o, s;
+	uint32_t part_actual;
 	bool subpart_found;
 
-	if (!part_header || ( !offset && !size && !part_actual)) {
+	if (!part_header || ( !offset && !size && !part_actualp)) {
 		prlog(PR_ERR, "FLASH: invalid parameters: ph %p of %p sz %p "
-		      "tsz %p\n", part_header, offset, size, part_actual);
+		      "tsz %p\n", part_header, offset, size, part_actualp);
 		return OPAL_PARAMETER;
 	}
 
@@ -68,7 +69,7 @@  int flash_subpart_info(void *part_header, uint32_t header_len,
 	      eyecatcher);
 
 	subpart_found = false;
-	*part_actual = 0;
+	part_actual = 0;
 	for (i = 0; i < FLASH_HOSTBOOT_TOC_MAX_ENTRIES; i++) {
 
 		ec = be32_to_cpu(header->toc[i].ec);
@@ -97,8 +98,8 @@  int flash_subpart_info(void *part_header, uint32_t header_len,
 		 * Subpartitions content are different, but multiple toc entries
 		 * may point to the same subpartition.
 		 */
-		if (ALIGN_UP(o + s, FLASH_SUBPART_HEADER_SIZE) > *part_actual)
-			*part_actual = ALIGN_UP(o + s, FLASH_SUBPART_HEADER_SIZE);
+		if (ALIGN_UP(o + s, FLASH_SUBPART_HEADER_SIZE) > part_actual)
+			part_actual = ALIGN_UP(o + s, FLASH_SUBPART_HEADER_SIZE);
 
 		if (ec == subid) {
 			if (offset)
@@ -112,5 +113,7 @@  int flash_subpart_info(void *part_header, uint32_t header_len,
 		prerror("FLASH: flash subpartition not found.\n");
 		return OPAL_RESOURCE;
 	}
+	if (part_actualp)
+		*part_actualp = part_actual;
 	return OPAL_SUCCESS;
 }
diff --git a/core/flash.c b/core/flash.c
index 6d6e3547..065fde29 100644
--- a/core/flash.c
+++ b/core/flash.c
@@ -428,9 +428,13 @@  int flash_register(struct blocklevel_device *bl)
 	if (rc)
 		return rc;
 
+	if (!name)
+		name = "(unnamed)";
+
 	prlog(PR_INFO, "FLASH: registering flash device %s "
 			"(size 0x%llx, blocksize 0x%x)\n",
-			name ?: "(unnamed)", size, block_size);
+			name, size, block_size);
+
 	flash = malloc(sizeof(struct flash));
 	if (!flash) {
 		prlog(PR_ERR, "FLASH: Error allocating flash structure\n");