Bootloader boots to wrong entry point if init_sflash fails

Tip / Sign in to post questions, reply, level up, and achieve exciting badges. Know more

cross mob
Anonymous
Not applicable

When the bootloader attempts to load a new image from external serial flash, if init_sflash() fails, then the bootloader will attempt to run a program using an entry point that is unititialized garbage from the stack. The device won't boot.

What is happening:

1) load_program() [apps/waf/bootloader/bootloader.c] returns *new_entry_point to main().

2) load_program() calls wiced_waf_app_load() [WICED/platform/MCU/wiced_waf_common.c] to generate the value for *new_entry_point.

3) wiced_waf_app_load() creates a local structure, header, without explicitly initializing it to 0.

4) wiced_waf_app_load() calls wiced_apps_read() [WICED/platform/MCU/wiced_aps_common.c] to populate header with the correct values from the file,

5) but if init_sflash fails, wiced_apps_read() makes no changes to header and returns the uninitialized block of memory and a -1 instead of WICED_SUCCESS.

6) wiced_waf_app_load() does not check the return code from wiced_apps_read() and assigns what becomes *new_entry_point to header.entry which is just random unitialized garbage from the stack.

There are two things to fix in wiced_waf_app_load(): It needs to handle any error when it calls wiced_apps_read(), and It would be a good idea to initialize the local header structure to a set of known reasonable constants or zero.

Note: load_program() does not check the return code from wiced_waf_app_load(), but as long as *new_entry_point does not get assigned to garbage this should not matter too much because main() will start the program at a valid entry point.

wiced_result_t wiced_waf_app_load( const image_location_t* app_header_location, uint32_t* destination )

{...

   elf_header_t header;

   memset( &header, 0, sizeof( elf_header_t ) );// Initialization added.

...

   if ( app_header_location->id == EXTERNAL_FIXED_LOCATION )

    {

...

        /* Read the image header */

//        wiced_apps_read( app_header_location, (uint8_t*) &header, 0, sizeof( header ) ); // Original

//Replacement:

        if ( wiced_apps_read( app_header_location, (uint8_t*) &header, 0, sizeof( header ) )  != WICED_SUCCESS )

        {

            return WICED_ERROR;

        }

...

        result = WICED_SUCCESS;

    }

// This is confusing: Since result is always set to WICED_SUCCESS at the end of the previous "if",

// the following "if" is exactly the same as inserting "*(uint32_t *) destination = header.entry;" at the end of the previous "if" clause.

    if ( result == WICED_SUCCESS )

    {

        *(uint32_t *) destination = header.entry;

    }

    return result;

}

4 Replies
Anonymous
Not applicable

Hello Eric,

I have checked the code. I do agree that we are not checking the return status of wiced_apps_read() function and we are proceeding assigning the entry point in wiced_waf_app_load() function. This will result in unknown entry point as the header is not initialized to zero/known value.

I will raise this internally and will try to correct it in next release. At present you can do the following modification and proceed further with your development

1) check the written value of wiced_apps_read() and return the wiced_waf_app_load() to load_program() function. If the return value is success then only go a head.

- or -

2) Initialized the values of header to zero so that you will have some control on the booting entry point.

I completely accept your explanation, after some couple of test I will raise this internally and make sure it will be corrected in next release.

grgaajai

rash wrote:

Hello Eric,

I have checked the code. I do agree that we are not checking the return status of wiced_apps_read() function and we are proceeding assigning the entry point in wiced_waf_app_load() function. This will result in unknown entry point as the header is not initialized to zero/known value.

I will raise this internally and will try to correct it in next release. At present you can do the following modification and proceed further with your development

1) check the written value of wiced_apps_read() and return the wiced_waf_app_load() to load_program() function. If the return value is success then only go a head.

- or -

2) Initialized the values of header to zero so that you will have some control on the booting entry point.

I completely accept your explanation, after some couple of test I will raise this internally and make sure it will be corrected in next release.

grga ajai

rash

wiced-studio 5.0 does not include this fix.

Something wrong in your internal communication system.

0 Likes

axel.lin_1746341 wrote:

rash wrote:

Hello Eric,

I have checked the code. I do agree that we are not checking the return status of wiced_apps_read() function and we are proceeding assigning the entry point in wiced_waf_app_load() function. This will result in unknown entry point as the header is not initialized to zero/known value.

I will raise this internally and will try to correct it in next release. At present you can do the following modification and proceed further with your development

1) check the written value of wiced_apps_read() and return the wiced_waf_app_load() to load_program() function. If the return value is success then only go a head.

- or -

2) Initialized the values of header to zero so that you will have some control on the booting entry point.

I completely accept your explanation, after some couple of test I will raise this internally and make sure it will be corrected in next release.

grga ajai

rash

wiced-studio 5.0 does not include this fix.

Something wrong in your internal communication system.

rash

SDK-5.2.0 does not include this fix.

I'm so surprised that even you confirmed the issue but the fix is still not apply to new sdk.

Could you check with your team again and confirm that someone fix this in next release?

grgaajai

0 Likes

axel.lin_1746341 wrote:

rash

SDK-5.2.0 does not include this fix.

I'm so surprised that even you confirmed the issue but the fix is still not apply to new sdk.

I do believe something very wrong in cypress internal communication.

Can someone fix this in next release?

grga ajai

Still not fix in sdk-6.0.0.

rashgrgaajai

0 Likes