Draft
Conversation
Includes a genuine bug in multicore_doorbell_claim() which seemed to use the wrong bit for the second core.
Contributor
Author
|
The SHA-256 errors are OOB reads in diff --git a/src/rp2_common/pico_sha256/sha256.c b/src/rp2_common/pico_sha256/sha256.c
index 91009c8..53dfaab 100644
--- a/src/rp2_common/pico_sha256/sha256.c
+++ b/src/rp2_common/pico_sha256/sha256.c
@@ -75,6 +75,8 @@ int pico_sha256_start_blocking_until(pico_sha256_state_t *state, enum sha256_end
return rc;
}
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wanalyzer-out-of-bounds"
static void write_to_hardware(pico_sha256_state_t *state, const uint8_t *data, size_t data_size_bytes) {
if (state->channel >= 0) {
dma_channel_wait_for_finish_blocking(state->channel);
@@ -111,6 +113,7 @@ static void write_to_hardware(pico_sha256_state_t *state, const uint8_t *data, s
}
}
}
+#pragma GCC diagnostic pop
static void update_internal(pico_sha256_state_t *state, const uint8_t *data, size_t data_size_bytes) {
assert(state->locked);...but there is a lot of other scary buffer handling in there which we want to be analysed. |
Contributor
Same thing as #2667 ? |
Contributor
Author
|
Yes, same fix. |
Contributor
|
i will take a look at this some more - not keen on the addition of panic to the alarm pool stuff as it introduces code bloat for pretty much everything |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These are from building
kitchen_sinkwith-fanalyzeron GCC 14.3.pico_time+ 1on claim bit for second core inmulticore_doorbell_claim()-- besides being OOB this also seems to just be looking at the wrong bit as it doesn't match the one used inmulticore_doorbell_unclaim()local_rng_stateininitialise_rand-- this looked like it was trying to be deliberately uninitialised but this doesn't garner much entropy for a stack variable, and it's undefined behaviourThere are some remaining errors in the SHA-256 which look pessimistic to me. It would still be good to clean that up so that people can use
-fanalyzerwhen building against the SDK.