[PATCH 1/2] state: backend_storage: deal gracefully with runtime bucket corruption

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Corrupting an already selected bucket and then reading it again will
crash barebox when it attempts the refresh:

  barebox$ state -l
  barebox$ mw -d /dev/eeprom0.state 0 0x42
  barebox$ state -l
  ERROR: state: No meta data header found
  state: Using bucket 1@0x00000040
  unable to handle NULL pointer dereference at address 0x00000000
  pc : [<4fe4f1ea>]    lr : [<4fe0bcb1>]
  sp : 4ffefd5c  ip : 00000000  fp : 2ff68f04
  r10: 4ffefdc8  r9 : 4b434d63  r8 : 30155f50
  r7 : 00000024  r6 : 2ff68b60  r5 : 2ff68e90  r4 : 00000000
  r3 : 00000024  r2 : 00000024  r1 : 30155f50  r0 : 00000000
  Flags: Nzcv  IRQs off  FIQs off  Mode SVC_32
  WARNING: [<4fe4f1ea>] (memcmp+0x14/0x1a) from [<4fe0bcb1>] (bucket_refresh.isra.0+0x4d/0x78)
  WARNING: [<4fe0bcb1>] (bucket_refresh.isra.0+0x4d/0x78) from [<4fe0be1d>] (state_storage_read+0xd1/0x104)
  WARNING: [<4fe0be1d>] (state_storage_read+0xd1/0x104) from [<4fe0a5bd>] (state_do_load+0x1d/0x78)
  WARNING: [<4fe0a5bd>] (state_do_load+0x1d/0x78) from [<4fe04137>] (execute_command+0x23/0x4c)

The memcmp called here is an optimization to skip I/O if the used bucket
and the one to be refreshed compare equal. Unfortunately, if the now
corrupt bucket was previously the used one, bucket->len will hold the
old value and we'll run into a NULL pointer dereference.

While this is quite inconvenient, it appears it doesn't affect
correctness: after the reset, the corrupt bucket will be refreshed
as expected.

Improve upon this by setting the length to zero when we are NULLing the
buffer. The zero length of the corrupted bucket will then compare unequal
to used_bucket->len in bucket_refresh() and ensure we will always refresh
the buffer if it becomes corrupted without an intermittent reset.

Fixes: 238008b4bd8f ("state: Drop cache bucket")
Cc: Enrico Jörns <ejo@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/state/backend_storage.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/state/backend_storage.c b/common/state/backend_storage.c
index fca887e93fa3..fe7e89e8fb39 100644
--- a/common/state/backend_storage.c
+++ b/common/state/backend_storage.c
@@ -192,6 +192,7 @@ int state_storage_read(struct state_backend_storage *storage,
 		/* Free buffer from the unused buckets */
 		free(bucket->buf);
 		bucket->buf = NULL;
+		bucket->len = 0;
 	}
 
 	/*
@@ -204,6 +205,7 @@ int state_storage_read(struct state_backend_storage *storage,
 
 	/* buffer from the used bucket is passed to the caller, do not free */
 	bucket_used->buf = NULL;
+	bucket_used->len = 0;
 
 	return 0;
 }
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox