Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change DCT read access to lock()/unlock() or read-with-copy #1307

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions bootloader/makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ LIB_DIRS += $(dir $(LIB_DEPS))

export COMPILE_LTO=y

export BOOTLOADER_MODULE=1

include ../build/platform-id.mk

# Target this makefile is building.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
#define CRC_TYPE uint32_t
#endif

#include "../hal/src/photon/dct_hal.c"
#include "../hal/src/photon/dct_hal.cpp"
4 changes: 2 additions & 2 deletions bootloader/src/photon/sources.mk
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ BOOTLOADER_SRC_COREV2_PATH = $(BOOTLOADER_MODULE_PATH)/src/photon
include $(BOOTLOADER_MODULE_PATH)/src/stm32f2xx/sources.mk

CSRC += $(call target_files,$(BOOTLOADER_SRC_COREV2_PATH)/,*.c)

CPPSRC += $(call target_files,$(BOOTLOADER_SRC_COREV2_PATH)/,*.cpp)

HAL_LIB_COREV2 = $(HAL_SRC_COREV2_PATH)/lib
WICED_LIBS = Platform_$(PLATFORM_NET) SPI_Flash_Library_$(PLATFORM_NET)

WICED_LIB_FILES = $(addprefix $(HAL_LIB_COREV2)/,$(addsuffix .a,$(WICED_LIBS)))
WICED_LIB_FILES = $(HAL_LIB_COREV2)/FreeRTOS/STM32F2xx_bootloader.a
WICED_LIB_FILES = $(HAL_LIB_COREV2)/FreeRTOS/STM32F2xx_bootloader.a $(HAL_LIB_COREV2)/FreeRTOS/WICED.a

LIBS_EXT += -Wl,--whole-archive $(WICED_LIB_FILES) -Wl,--no-whole-archive
4 changes: 3 additions & 1 deletion bootloader/src/stm32f2xx/button.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ int BUTTON_Debounce() {
}

void BUTTON_Init_Ext() {
const button_config_t* conf = (const button_config_t*)dct_read_app_data(DCT_MODE_BUTTON_MIRROR_OFFSET);
const button_config_t* conf = (const button_config_t*)dct_read_app_data_lock(DCT_MODE_BUTTON_MIRROR_OFFSET);

if (conf->active == 0xAA && conf->debounce_time == 0xBBCC) {
//int32_t state = HAL_disable_irq();
Expand All @@ -74,6 +74,8 @@ void BUTTON_Init_Ext() {
//HAL_enable_irq(state);
}

dct_read_app_data_unlock(DCT_MODE_BUTTON_MIRROR_OFFSET);

if (BUTTON_Debounce())
TIM_ITConfig(TIM2, TIM_IT_CC1, ENABLE);
}
Expand Down
12 changes: 12 additions & 0 deletions bootloader/src/stm32f2xx/concurrent_hal.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@

int os_thread_yield() {
return 0;
}

void __flash_acquire() {

}

void __flash_release() {

}
3 changes: 2 additions & 1 deletion bootloader/src/stm32f2xx/led_signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ static uint32_t led_signal_color(int signal, const uint8_t* data) {

void get_led_theme_colors(uint32_t* firmware_update, uint32_t* safe_mode, uint32_t* dfu_mode) {
// Check if theme data is initialized in DCT
const uint8_t* d = (const char*)dct_read_app_data(DCT_LED_THEME_OFFSET);
const uint8_t* d = (const char*)dct_read_app_data_lock(DCT_LED_THEME_OFFSET);
if (d && *d == LED_SIGNAL_THEME_VERSION) {
*firmware_update = led_signal_color(LED_SIGNAL_FIRMWARE_UPDATE, d);
*safe_mode = led_signal_color(LED_SIGNAL_SAFE_MODE, d);
*dfu_mode = led_signal_color(LED_SIGNAL_DFU_MODE, d);
}
dct_read_app_data_unlock(DCT_LED_THEME_OFFSET);
}
29 changes: 0 additions & 29 deletions hal/src/photon/dct_hal.c

This file was deleted.

79 changes: 79 additions & 0 deletions hal/src/photon/dct_hal.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#include "dct_hal.h"
#include "wiced_result.h"
#include "wiced_dct_common.h"
#include "waf_platform.h"
#include "wiced_framework.h"
#include "module_info.h"
#include "service_debug.h"
#include "hal_irq_flag.h"
#include "atomic_flag_mutex.h"

using DCTLock = AtomicFlagMutex<os_result_t, os_thread_yield>;

static DCTLock dctLock;

extern uint32_t Compute_CRC32(const uint8_t *pBuffer, uint32_t bufferSize);

// Compatibility crc32 function for WICED
extern "C" uint32_t crc32(uint8_t* pdata, unsigned int nbytes, uint32_t crc) {
uint32_t crcres = Compute_CRC32(pdata, nbytes);
// We need to XOR computed CRC32 value with 0xffffffff again as it was already xored in Compute_CRC32
// and apply passed crc value instead.
crcres ^= 0xffffffff;
crcres ^= crc;
return crcres;
}

int dct_read_app_data_copy(uint32_t offset, void* ptr, size_t size)
{
void* p = NULL;
int result = 0;
if (ptr && wiced_dct_read_lock(&p, WICED_FALSE, DCT_APP_SECTION, offset, 0) == WICED_SUCCESS)
{
memcpy(ptr, p, size);
}
else
{
result = 1;
}
wiced_dct_read_unlock(NULL, WICED_FALSE);

return result;
}

const void* dct_read_app_data_lock(uint32_t offset)
{
void* ptr = NULL;
wiced_dct_read_lock(&ptr, WICED_FALSE, DCT_APP_SECTION, offset, 0);
return (const void*)ptr;
}

int dct_read_app_data_unlock(uint32_t offset)
{
return wiced_dct_read_unlock(NULL, WICED_FALSE);
}

const void* dct_read_app_data(uint32_t offset) {
wiced_dct_lock();
const char* ptr = ((const char*)wiced_dct_get_current_address(DCT_APP_SECTION) + offset);
wiced_dct_unlock();
return ptr;
}

int dct_write_app_data(const void* data, uint32_t offset, uint32_t size) {
int res = wiced_dct_write(data, DCT_APP_SECTION, offset, size);

return res;
}

wiced_result_t wiced_dct_lock()
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add HAL_IsISR() check here?

dctLock.lock();
return WICED_SUCCESS;
}

wiced_result_t wiced_dct_unlock()
{
dctLock.unlock();
return WICED_SUCCESS;
}
Binary file modified hal/src/photon/lib/FreeRTOS/STM32F2xx.a
Binary file not shown.
Binary file modified hal/src/photon/lib/FreeRTOS/STM32F2xx_bootloader.a
Binary file not shown.
Binary file modified hal/src/photon/lib/FreeRTOS/WICED.a
Binary file not shown.
Binary file modified hal/src/photon/lib/STM32F2xx_Peripheral_Libraries.a
Binary file not shown.
23 changes: 17 additions & 6 deletions hal/src/photon/softap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ int dns_resolve_query(const char* query)
int result = dns_resolve_query_default(query);
if (result<=0)
{
const char* valid_queries = (const char*) dct_read_app_data(DCT_DNS_RESOLVE_OFFSET);
const char* valid_queries = (const char*) dct_read_app_data_lock(DCT_DNS_RESOLVE_OFFSET);
result = resolve_dns_query(query, valid_queries);
dct_read_app_data_unlock(DCT_DNS_RESOLVE_OFFSET);
}
return result;
}
Expand Down Expand Up @@ -524,7 +525,9 @@ int decrypt(char* plaintext, int max_plaintext_len, char* hex_encoded_ciphertext
hex_decode(buf, len, hex_encoded_ciphertext);

// reuse the hex encoded buffer
int plaintext_len = decrypt_rsa(buf, fetch_device_private_key(), (uint8_t*)plaintext, max_plaintext_len);
const uint8_t *key = fetch_device_private_key(1); // fetch and lock private key data
int plaintext_len = decrypt_rsa(buf, key, (uint8_t*)plaintext, max_plaintext_len);
fetch_device_private_key(0); // unlock private key data
return plaintext_len;
}

Expand Down Expand Up @@ -778,7 +781,7 @@ class PublicKeyCommand : public JSONCommand {
void produce_response(Writer& writer, int result) {
// fetch public key
const int length = EXTERNAL_FLASH_SERVER_PUBLIC_KEY_LENGTH;
const uint8_t* data = fetch_device_public_key();
const uint8_t* data = fetch_device_public_key(1);
write_char(writer, '{');
if (data) {
writer.write("\"b\":\"");
Expand All @@ -794,6 +797,8 @@ class PublicKeyCommand : public JSONCommand {
result = 1;
}

fetch_device_public_key(0);

write_json_int(writer, "r", result);
write_char(writer, '}');
}
Expand Down Expand Up @@ -899,34 +904,38 @@ extern "C" bool fetch_or_generate_setup_ssid(wiced_ssid_t* SSID);
* @return true if the device code was generated.
*/
bool fetch_or_generate_device_code(wiced_ssid_t* SSID) {
const uint8_t* suffix = (const uint8_t*)dct_read_app_data(DCT_DEVICE_CODE_OFFSET);
const uint8_t* suffix = (const uint8_t*)dct_read_app_data_lock(DCT_DEVICE_CODE_OFFSET);
int8_t c = (int8_t)*suffix; // check out first byte
bool generate = (!c || c<0);
uint8_t* dest = SSID->value+SSID->length;
SSID->length += DEVICE_CODE_LEN;
if (generate) {
dct_read_app_data_unlock(DCT_DEVICE_CODE_OFFSET);
random_code(dest, DEVICE_CODE_LEN);
dct_write_app_data(dest, DCT_DEVICE_CODE_OFFSET, DEVICE_CODE_LEN);
}
else {
memcpy(dest, suffix, DEVICE_CODE_LEN);
dct_read_app_data_unlock(DCT_DEVICE_CODE_OFFSET);
}
return generate;
}

const int MAX_SSID_PREFIX_LEN = 25;

bool fetch_or_generate_ssid_prefix(wiced_ssid_t* SSID) {
const uint8_t* prefix = (const uint8_t*)dct_read_app_data(DCT_SSID_PREFIX_OFFSET);
const uint8_t* prefix = (const uint8_t*)dct_read_app_data_lock(DCT_SSID_PREFIX_OFFSET);
uint8_t len = *prefix;
bool generate = (!len || len>MAX_SSID_PREFIX_LEN);
if (generate) {
dct_read_app_data_unlock(DCT_SSID_PREFIX_OFFSET);
strcpy((char*)SSID->value, "Photon");
SSID->length = 6;
dct_write_app_data(SSID, DCT_SSID_PREFIX_OFFSET, SSID->length+1);
}
else {
memcpy(SSID, prefix, DCT_SSID_PREFIX_SIZE);
dct_read_app_data_unlock(DCT_SSID_PREFIX_OFFSET);
}
if (SSID->length>MAX_SSID_PREFIX_LEN)
SSID->length = MAX_SSID_PREFIX_LEN;
Expand Down Expand Up @@ -964,9 +973,11 @@ class SoftAPController {
if (result == WICED_SUCCESS)
{
if (memcmp(&expected, soft_ap, sizeof(expected))) {
wiced_dct_read_unlock( soft_ap, WICED_FALSE );
result = wiced_dct_write(&expected, DCT_WIFI_CONFIG_SECTION, OFFSETOF(platform_dct_wifi_config_t, soft_ap_settings), sizeof(wiced_config_soft_ap_t));
} else {
wiced_dct_read_unlock( soft_ap, WICED_FALSE );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 👍 I've just checked other places where we use non-copying variant of wiced_dct_read_lock(), and haven't found anything suspicious.

}
wiced_dct_read_unlock( soft_ap, WICED_FALSE );
}
return result;
}
Expand Down
3 changes: 3 additions & 0 deletions hal/src/photon/wiced/platform/MCU/wiced_dct_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ wiced_result_t wiced_dct_restore_factory_reset ( void );
void* wiced_dct_get_current_address ( dct_section_t section );
wiced_result_t wiced_erase_non_current_dct (void);

extern wiced_result_t wiced_dct_lock(void);
extern wiced_result_t wiced_dct_unlock(void);

#ifdef __cplusplus
} /*extern "C" */
#endif
Loading