Possible OTA bug?

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

cross mob
Anonymous
Not applicable

In OTA function  (sdk  3.5.2 , 3.7.0 ....4)

wiced_apps_set_size   allways increment (by value 34)  image_location_t  place address in spi flash. So, after call  wiced_apps_set_size (firmware  upgrade) more time, it overwrite other data in spi flash. It seems to me that bug. If data are in next sector (possible factory restore image) then after about 4096/34-8 = 112 times this data will be corrupted and firmare update can work incorectly, because  image_location data  will be bad, because it try write in not clear spi flash place.

My test function.

void test(void)

{

    wiced_app_t app;

    if (1)

    {

        uint32_t file_size=199924;//1151444

        uint32_t    current_size;

        if (wiced_framework_app_open( DCT_APP0_INDEX, &app ) != WICED_SUCCESS)

        {

            printf("ERR1\n");

            return ;

        }

        if (wiced_framework_app_get_size( &app, &current_size ) != WICED_SUCCESS)

        {

            printf("ERR2\n");

            return ;

        }

        if (wiced_framework_app_set_size( &app, file_size) != WICED_SUCCESS)

        {

            mini_printf("ERR3\n");

            return ;

        }/**/

        if (wiced_framework_app_get_size( &app, &current_size ) != WICED_SUCCESS)

        {

            printf("ERR4\n");

            return ;

        }

        if ( current_size < file_size  )

        {

            printf("Error setting application size!!\n");

            return ;

        }

    }

}

Message was edited. Additional note 3 at last repply.

6 Replies
AxLi_1746341
Level 7
Level 7
10 comments on KBA 5 comments on KBA First comment on KBA

darius1 wrote:

In OTA function  (sdk  3.5.2 , 3.7.0 ....4)

wiced_apps_set_size   allways increment (by value 34)  image_location_t  place address in spi flash. So, after call 

Hi darius1

Do you know which part of the code doing "increment image_location_t"?.

In case I might misunderstand the issue, how do you find the image_location_t  place address in spi flash  incremented?

Axel

Anonymous
Not applicable

!@Hi Axel,

wiced_result_t wiced_waf_app_set_size(wiced_app_t* app, uint32_t size)

{

    if ( wiced_apps_set_size( &app->app_header_location, size ) != WICED_SUCCESS )

    {

        return WICED_ERROR;

    }

    return wiced_dct_set_app_header_location( app->app_id, &app->app_header_location );

}

In part  wiced_apps_set_size( &app->app_header_location, size )  increment

In part wiced_dct_set_app_header_location( app->app_id, &app->app_header_location );  new header location in spi flash is saved in dct.

But i must correct it self. Not "always" increment, but only if new size bigger by 1sector >= 4096. (its real situation new firmware always bigger). But if new size bigger I see 3 problems:

1.  New size are reserved in internal spi flash header sections[8] - so after 8 bigger firmware - overload sections array. I not sure, that sdk can correctly work with this sections(working ok). Fix I reported her: wiced_apps_set_size (OTA) fix

2. spi flash write (sflash_write) function not with all chips working correctly (maybe only correctly, but slowly, which using max write size 1 byte ). So new headers in spi flash can save incorrectly. It occurs if  spi flash  ( (address & 0xFF)+size-1) >= 0x100. (if write page 256 bytes). Fix I reported her: sflash_write error fix (OTA)

3. New header are wrote in new place off spi flash (because new place must be clear)  so after som time this headers overwrite factory restore image, which is in next sector. No way back to ok state. Need rewrite(clear) app lut sector in spi flash.

I think possible to do workaround. Need first firmware to do  very big (or maybe save this big fake firmware in factory to reserve size, or in lut possible reserve one biggest size). But we lost read-save time.

Additional notes:

1. OTA   make command always must have download_apps, which clear  app sector in spi flash, to save new header.

2. In make file must defined at least one application  FR_APP, DCT_IMAGE...  if not  defined then firmware image overwrite app lut headers sector in spiflash - (bug)

3. If dct are restored then  lost information about new header positions in spi flash. New headers are writes in to not clear place. If new firmware file size  not same, then ota  stop working ok and  update process will corrupt program in flash. It seriously bug. Possible solution can be clear and restore headers section in spi flash at factory restore. But it not simple to do, because need get in restore struct information about application, dct, wifi firmare etc.  sizes, like in APPS.bin file which are compiled when download_apps set in make string.

0 Likes

Darius Babrauskas wrote:

if new size bigger by 1sector >= 4096. (its real situation new firmware always bigger). But if new size bigger I see 3 problems:

1.  New size are reserved in internal spi flash header sections[8] - so after 8 bigger firmware - overload sections array. I not sure, that sdk can correctly work with this sections(working ok). Fix I reported her: wiced_apps_set_size (OTA) fix

2. spi flash write (sflash_write) function not with all chips working correctly (maybe only correctly, but slowly, which using max write size 1 byte ). So new headers in spi flash can save incorrectly. It occurs if  spi flash  ( (address & 0xFF)+size-1) >= 0x100. (if write page 256 bytes). Fix I reported her: sflash_write error fix (OTA)

3. New header are wrote in new place off spi flash (because new place must be clear)  so after som time this headers overwrite factory restore image, which is in next sector. No way back to ok state. Need rewrite(clear) app lut sector in spi flash.

I think possible to do workaround. Need first firmware to do  very big (or maybe save this big fake firmware in factory to reserve size, or in lut possible reserve one biggest size). But we lost read-save time.

New firmware usually bigger than the old one due to new features or bug fixes.

The workaroud to build a big fake firmware is a bad thing.

Burning a fake big firmware takes time, it means money in production.

Any comment from cypress team?

I can confirm this is a bug in SDK.

mifokausikrash

0 Likes
AxLi_1746341
Level 7
Level 7
10 comments on KBA 5 comments on KBA First comment on KBA

In WICED/platform/MCU/wiced_apps_common.h

typedef struct

{  

        uint8_t     count;          /* Number of entries */

#ifndef BOOTLOADER_APP_LUT_NO_SECURE_FLAG

        uint8_t     secure;         /* Is this app secure (Signed/Encrypted) or not - Added in SDK-3.4.0 */

#endif

        app_entry_t sectors[8];     /* An app can have a maximum of 8 amendments (for simplicity) */

}app_header_t;

""" An app can have a maximum of 8 amendments (for simplicity) """

This looks scary especially it's not document anywhere.

In additional, the 9th amendment causes array out of bound access. (in wiced_apps_get_size().)

The OTA1 really needs some love!

mifokausikrash

AxLi_1746341
Level 7
Level 7
10 comments on KBA 5 comments on KBA First comment on KBA

Darius Babrauskas wrote:

In OTA function  (sdk  3.5.2 , 3.7.0 ....4)

wiced_apps_set_size   allways increment (by value 34)  image_location_t  place address in spi flash. So, after call  wiced_apps_set_size (firmware  upgrade) more time, it overwrite other data in spi flash. It seems to me that bug. If data are in

Hi,

I think the increment 34 bytes is because

In wiced_apps_find_empty_location()

            if ( existing_header_location.detail.external_fixed.location >= new_header_location->detail.external_fixed.location )

            {

                new_header_location->detail.external_fixed.location = existing_header_location.detail.external_fixed.location + sizeof(app_header_t);

            }

The sizeof(app_header_t) is 34.

However, I don't understand above code: why set the location to existing_header_location.detail.external_fixed.location + sizeof(app_header_t)?

Can someone help to explain it?

0 Likes
Anonymous
Not applicable

Hi Axel,

>However, I don't understand above code: why set the location to existing_header_location.detail.external_fixed.location +

>sizeof(app_header_t)?

Answer her ... for ( index = 0; index < DCT_MAX_APP_COUNT; index++ )

it find last reserved sector in all applications, because possible update not only firmware (APP0) but and others APP1 APP2, WIFI.., DCT..  To save new header need new clear sflash place, because to save in same place before need clear sector. Best solution use 2 sectors like dct for save apps.bin.

But her we have problem, that after some update we get fragmentation and limits (8 differ sector position and sizes). I reported fix which not overload section array if using only one update(APP0). But if use multi update we have sectors fragmentation.

So after 4  sequence increment updates everload 8 sections limits and system stop updates. Exmp.:

1. update APP0 (same section)

2. update APP1 (section++  fragmentation)

3. update APP0 (section++ fragmentation)

4. update APP1 (section++ fragmentation)

5. update APP0 (section++ fragmentation)

6. update APP1(section++ fragmentation)

7. update APP0 (section++ fragmentation)

8. update APP1(section++ fragmentation)

My solution is to do factory restore with restore app header  in sflash. Simplest solution to fake biggest app0 file, but lost,  like you say flashing, time.