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@xxxxxxxxxxxxxx>
Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
