STMicroelectronics / stm32h7xx-hal-driver

Provides the STM32Cube MCU Component "hal_driver" of the STM32H7 series.
BSD 3-Clause "New" or "Revised" License
97 stars 43 forks source link

HASH_Start_IT fails when called with buffer size < 4 while in HAL_HASH_PHASE_PROCESS phase #62

Closed mariuszste closed 4 months ago

mariuszste commented 5 months ago

Describe the set-up

Describe the bug HASH_Start_IT silently fails when called with Size < 4 while in HAL_HASH_PHASE_PROCESS phase. The interrupt handler is never called, no error returned

How To Reproduce

  1. Indicate the global behavior of your application project Hashing data of arbitrary length

  2. The modules that you suspect to be the cause of the problem (Driver, BSP, MW ...) stm32h7xx_hal_hash.c HASH_Start_IT

  3. The use case that generates the problem I'm reading and hashing data in chunks, the last chunk can be any size. Let's say the chunk size is 64 bytes and I want to hash 66 bytes of data. That is 64 + 2

  4. How we can reproduce the problem

this should give a general idea of what I'm trying to do.

HASH_HandleTypeDef hhash;
/////////
void example() {
    HAL_HASH_DeInit(&hhash);
    hhash.Init.DataType = HASH_DATATYPE_8B;
    HAL_HASH_Init(&hhash);

    tx_semaphore_create(&sync, "HASH sync", 0);

    uint8_t buf[64];
    uint8_t out[32];
    memset(buf, 0, sizeof(buf));

    // chunk 1
    // This will switch the phase to HAL_HASH_PHASE_PROCESS, removing it will make the next call succeed
    HASH_Accumulate_IT(&hhash, buf, 64, HASH_ALGOSELECTION_SHA256);
    tx_semaphore_get(&sync, TX_WAIT_FOREVER);

    // chunk 2 - last
    // can also be 
    HASH_Start_IT(&hhash, buf, 2, out, HASH_ALGOSELECTION_SHA256);
    tx_semaphore_get(&sync, TX_WAIT_FOREVER); // this will hang because `HASH_RNG_IRQHandler` will never be called
}
/////////
void HAL_HASH_InCpltCallback(HASH_HandleTypeDef *hhash) {
    tx_semaphore_put(&sync);
}

void HAL_HASH_DgstCpltCallback(HASH_HandleTypeDef *hhash) {
    tx_semaphore_put(&sync);
}

Additional context polling_step will never be 1 if Size is < 4 so the entire if (polling_step == 1U) {} block is skipped and since hhash->Phase is not HAL_HASH_PHASE_READY there is basically no code left in the function. Something here is seriously wrong.

I assume this is a supported use case based on the comments from HAL_HASHEx_SHA256_Accmlt_IT and HAL_HASHEx_SHA256_Accmlt_End_IT since they are just aliases for HASH_Accumulate_IT and HASH_Start_IT respectively.

As far as I can tell this works just fine with the blocking variants, as well as DMA, only the interrupt variant is broken.

Screenshots N/A

ASEHSTM commented 5 months ago

ST Internal Reference: 184776

ASEHSTM commented 4 months ago

Hello @mariuszste,

Thank you for your contribution. It appears that the issue is indeed related to the buffer size parameter used in your HASH_Start_IT function call. The function HASH_Start_IT(&hhash, buf, 2, out, HASH_ALGOSELECTION_SHA256); is currently configured to process only the first 2 bytes of the buffer buf, as indicated by the size parameter. This means that it does not consider the remaining bytes.

To resolve this, you'll need to adjust the buffer pointer to point to the correct location where the remaining bytes start. In your case, if you have already processed 64 bytes, the API call should be adjusted as follows: HASH_Start_IT(&hhash, buf + 64, 2, out, HASH_ALGOSELECTION_SHA256);

With Regards,

mariuszste commented 4 months ago

To resolve this, you'll need to adjust the buffer pointer to point to the correct location where the remaining bytes start. In your case, if you have already processed 64 bytes, the API call should be adjusted as follows: HASH_Start_IT(&hhash, buf + 64, 2, out, HASH_ALGOSELECTION_SHA256);

No, the code is written this way just to demonstrate it. That's not the issue, it will break either way. I have data coming in chunks and the chunks are read into the same buffer one at a time and then hashed before reading the next one.

This means that it does not consider the remaining bytes.

well, that's the expected behaviour on the lust chunk since it can be smaller. The issue is that this doesn't actually happen when size is <4, no bytes are processed and the interrupt is never fired.

if need be, I can provide a better example showing that this works perfectly with the blocking and DMA variants but not the interrupt one.

ASEHSTM commented 4 months ago

Hello @mariuszste,

To assist you more effectively, could you please provide us with an example of your use case?

With Regards,

RJMSTM commented 4 months ago

Fixed in : 913697ca128abb608114f4d7b86ecb7a03a8f5be

mariuszste commented 4 months ago

Sorry for forgetting to make the POC :sweat_smile: Guess someone finally understood what I meant. Thanks