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.

Sign up for GitHub

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

lwip/wifi: Fix crashes during reinitialisation of wi-fi #88

Merged
merged 2 commits into from
Jan 26, 2024
Merged

lwip/wifi: Fix crashes during reinitialisation of wi-fi #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a576a27
lwip/wi-fi: Fix invalid memory access during wi-fi deinitialisation.
astalke Jan 10, 2024
e4cea8c
lwip/wi-fi: Make IRQ thread init and deinit safer
astalke Jan 10, 2024
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: 1 addition & 1 deletion wi-fi/hal/cyabs_rtos.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ cy_rslt_t cy_rtos_create_thread(cy_thread_t *thread, cy_thread_entry_fn_t entry_
}


void cy_rtos_exit_thread()
void cy_rtos_exit_thread(void)
{
cy_log_msg(CYLF_RTOS, CY_LOG_DEBUG, "cy_rtos_exit_thread\n");

Expand Down
68 changes: 45 additions & 23 deletions wi-fi/hal/cyhal_sdio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ static struct {
void *dmaptr;
addr_t dmaphys;

bool irq_enabled;
volatile bool irq_enabled;

handle_t irq_handle;
handle_t irq_lock;
Expand All @@ -74,7 +74,7 @@ static struct {
cyhal_sdio_irq_handler_t irq_handler;
void *irq_handler_arg;

bool irq_thread_finish;
volatile bool irq_thread_finish;
cy_thread_t irq_thread_id;
} sdio_common;

Expand Down Expand Up @@ -258,20 +258,23 @@ static void sdio_irq_thread(void *arg)

while (1) {
finish = sdio_common.irq_thread_finish;
if (finish)
if (finish) {
break;
}

val = *(sdio_common.base + int_status);
if (val & (1 << 8))
if ((val & (1 << 8)) != 0) {
break;
}

condWait(sdio_common.irq_cond, sdio_common.irq_lock, 0);
}

mutexUnlock(sdio_common.irq_lock);

if (finish)
if (finish) {
break;
}

// cy_log_msg(CYLF_SDIO, CY_LOG_DEBUG, "got SDIO IRQ\n");

Expand All @@ -296,6 +299,17 @@ static void sdio_irq_thread(void *arg)
cy_rtos_exit_thread();
}

/* NOTE: obj is ignored - state is kept in sdio_common */
cy_rslt_t cyhal_sdio_start_irq_thread(cyhal_sdio_t *obj)
{
cy_log_msg(CYLF_SDIO, CY_LOG_DEBUG, "cyhal_sdio_start_irq_thread\n");
if (cy_rtos_create_thread(&sdio_common.irq_thread_id, sdio_irq_thread, "SDIO_IRQ", NULL, 1024, 4, NULL) != CY_RSLT_SUCCESS) {
cy_log_msg(CYLF_SDIO, CY_LOG_ERR, "failed to start SDIO IRQ handler thread\n");
return CYHAL_SDIO_RSLT_ERR_BAD_PARAM;
}
return CY_RSLT_SUCCESS;
}


/* NOTE: obj is ignored - state is kept in sdio_common */
cy_rslt_t cyhal_sdio_init(cyhal_sdio_t *obj)
Expand Down Expand Up @@ -334,11 +348,6 @@ cy_rslt_t cyhal_sdio_init(cyhal_sdio_t *obj)
break;
}

if (cy_rtos_create_thread(&sdio_common.irq_thread_id, sdio_irq_thread, "SDIO_IRQ", NULL, 1024, 4, NULL) != CY_RSLT_SUCCESS) {
cy_log_msg(CYLF_SDIO, CY_LOG_ERR, "failed to start SDIO IRQ handler thread\n");
break;
}

if ((cyhal_utils_set_iomux(pctl_mux_csi_vsync, 1) < 0) || /* USDHC2_CLK */
(cyhal_utils_set_iomux(pctl_mux_csi_hsync, 1) < 0) || /* USDHC2_CMD */
(cyhal_utils_set_iomux(pctl_mux_csi_d0, 1) < 0) || /* USDHC2_DATA0 */
Expand Down Expand Up @@ -398,14 +407,10 @@ cy_rslt_t cyhal_sdio_init(cyhal_sdio_t *obj)


/* NOTE: obj is ignored - state is kept in sdio_common */
void cyhal_sdio_free(cyhal_sdio_t *obj)
void cyhal_sdio_stop_irq_thread(cyhal_sdio_t *obj)
{
cy_log_msg(CYLF_SDIO, CY_LOG_DEBUG, "cyhal_sdio_free\n");

/* reset SDIO and set SDIO clock freq to 400000000 / 2 / (256 * 2) = 390625 Hz */
reset_all(0x80, 0x1); /* SDCLKFS=0x80 DVS=0x1 */

if (sdio_common.irq_thread_id) {
cy_log_msg(CYLF_SDIO, CY_LOG_DEBUG, "cyhal_sdio_stop_irq_thread\n");
if (sdio_common.irq_thread_id != 0) {
/* request IRQ thread to finish */
mutexLock(sdio_common.irq_lock);
sdio_common.irq_thread_finish = true;
Expand All @@ -414,23 +419,40 @@ void cyhal_sdio_free(cyhal_sdio_t *obj)

/* wait for IRQ thread to finish */
cy_rtos_join_thread(&sdio_common.irq_thread_id);
sdio_common.irq_thread_id = 0;
}
}


/* NOTE: obj is ignored - state is kept in sdio_common */
void cyhal_sdio_free(cyhal_sdio_t *obj)
{
cy_log_msg(CYLF_SDIO, CY_LOG_DEBUG, "cyhal_sdio_free\n");

/* reset SDIO and set SDIO clock freq to 400000000 / 2 / (256 * 2) = 390625 Hz */
reset_all(0x80, 0x1); /* SDCLKFS=0x80 DVS=0x1 */

if (sdio_common.irq_handle)
if (sdio_common.irq_handle != 0) {
resourceDestroy(sdio_common.irq_handle);
if (sdio_common.irq_lock)
}
if (sdio_common.irq_lock != 0) {
resourceDestroy(sdio_common.irq_lock);
if (sdio_common.irq_cond)
}
if (sdio_common.irq_cond != 0) {
resourceDestroy(sdio_common.irq_cond);
}

if (sdio_common.cmd_lock)
if (sdio_common.cmd_lock != 0) {
resourceDestroy(sdio_common.cmd_lock);
}

if (sdio_common.dmaptr)
if (sdio_common.dmaptr != NULL) {
munmap(sdio_common.dmaptr, DMA_BUFFER_SIZE);
}

if (sdio_common.base)
if (sdio_common.base != NULL) {
munmap((void *)sdio_common.base, _PAGE_SIZE);
}

memset(&sdio_common, 0, sizeof(sdio_common));
}
Expand Down
2 changes: 2 additions & 0 deletions wi-fi/hal/cyhal_sdio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,15 @@ typedef void (*cyhal_sdio_irq_handler_t)(void *handler_arg, cyhal_sdio_irq_event
* @return The status of the init request
*/
cy_rslt_t cyhal_sdio_init(cyhal_sdio_t *obj);
cy_rslt_t cyhal_sdio_start_irq_thread(cyhal_sdio_t *obj);

/** Release the SDIO peripheral, not currently invoked. It requires further
* resource management.
*
* @param[in,out] obj The SDIO object
*/
void cyhal_sdio_free(cyhal_sdio_t *obj);
void cyhal_sdio_stop_irq_thread(cyhal_sdio_t *obj);

/** Configure the SDIO block.
*
Expand Down
6 changes: 6 additions & 0 deletions wi-fi/lwip/cybsp_wifi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,12 @@ static inline cy_rslt_t _cybsp_wifi_bus_init(void)
static inline void _cybsp_wifi_bus_detach(void)
{
#if defined(WIFI_MODE_SDIO)
/* `sdio_irq_thread` must be stopped before we detach bus, because `sdio_common.irq_handler`
* is called by `sdio_irq_thread` and it requires `whd_driver->bus_priv` that is freed by
* `whd_bus_sdio_detach`
* `cyhal_sdio_stop_irq_thread` can be safely called twice
*/
cyhal_sdio_stop_irq_thread(cybsp_get_wifi_sdio_obj());
astalke marked this conversation as resolved.
Show resolved Hide resolved
whd_bus_sdio_detach(whd_drv);
#elif defined(WIFI_MODE_SPI)
whd_bus_spi_detach(whd_drv);
Expand Down
19 changes: 17 additions & 2 deletions wi-fi/lwip/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ static struct {
handle_t cond;
handle_t tid;

uint8_t flags;
volatile uint8_t flags;
uint32_t idle_timeout;
uint32_t idle_current;
whd_ssid_t ssid;
Expand Down Expand Up @@ -234,9 +234,18 @@ static void wifi_ap_thread(void *arg)
break;
}

result = cyhal_sdio_start_irq_thread(cybsp_get_wifi_sdio_obj());
if (result != CY_RSLT_SUCCESS) {
wm_cy_log_msg(CYLF_MIDDLEWARE, CY_LOG_ERR, "can't start IRQ thread\n");
cybsp_free();
break;
}

result = cybsp_wifi_init_primary(&wifi_common.iface.whd_iface);
if (result != CY_RSLT_SUCCESS) {
astalke marked this conversation as resolved.
Show resolved Hide resolved
wm_cy_log_msg(CYLF_MIDDLEWARE, CY_LOG_ERR, "can't init Wi-Fi interface\n");
/* cyhal_sdio_stop_irq_thread can be safely called twice */
cyhal_sdio_stop_irq_thread(cybsp_get_wifi_sdio_obj());
astalke marked this conversation as resolved.
Show resolved Hide resolved
cybsp_free();
break;
}
Expand All @@ -247,6 +256,7 @@ static void wifi_ap_thread(void *arg)
if (result != CY_RSLT_SUCCESS) {
wm_cy_log_msg(CYLF_MIDDLEWARE, CY_LOG_ERR, "can't add Wi-Fi interface\n");
cybsp_wifi_deinit(wifi_common.iface.whd_iface);
cyhal_sdio_stop_irq_thread(cybsp_get_wifi_sdio_obj());
cybsp_free();
break;
}
Expand All @@ -256,6 +266,7 @@ static void wifi_ap_thread(void *arg)
wm_cy_log_msg(CYLF_MIDDLEWARE, CY_LOG_ERR, "can't bring up Wi-Fi interface\n");
cy_lwip_remove_interface(&wifi_common.iface);
cybsp_wifi_deinit(wifi_common.iface.whd_iface);
cyhal_sdio_stop_irq_thread(cybsp_get_wifi_sdio_obj());
cybsp_free();
break;
}
Expand All @@ -266,6 +277,7 @@ static void wifi_ap_thread(void *arg)
cy_lwip_network_down(&wifi_common.iface);
cy_lwip_remove_interface(&wifi_common.iface);
cybsp_wifi_deinit(wifi_common.iface.whd_iface);
cyhal_sdio_stop_irq_thread(cybsp_get_wifi_sdio_obj());
cybsp_free();
break;
}
Expand All @@ -276,6 +288,7 @@ static void wifi_ap_thread(void *arg)
cy_lwip_network_down(&wifi_common.iface);
cy_lwip_remove_interface(&wifi_common.iface);
cybsp_wifi_deinit(wifi_common.iface.whd_iface);
cyhal_sdio_stop_irq_thread(cybsp_get_wifi_sdio_obj());
cybsp_free();
break;
}
Expand All @@ -295,6 +308,7 @@ static void wifi_ap_thread(void *arg)
cy_lwip_network_down(&wifi_common.iface);
cy_lwip_remove_interface(&wifi_common.iface);
cybsp_wifi_deinit(wifi_common.iface.whd_iface);
cyhal_sdio_stop_irq_thread(cybsp_get_wifi_sdio_obj());
cybsp_free();
}

Expand Down Expand Up @@ -326,8 +340,9 @@ static int wifi_ap_start(void)
return -1;
}

while (wifi_common.flags == 0)
while (wifi_common.flags == 0) {
condWait(wifi_common.cond, wifi_common.lock, 0);
}

flags = wifi_common.flags;

Expand Down
6 changes: 5 additions & 1 deletion wi-fi/whd/whd_management.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,11 @@ uint32_t whd_wifi_off(whd_interface_t ifp)
}

/* Set wlc down before turning off the device */
CHECK_RETURN(whd_wifi_set_ioctl_buffer(ifp, WLC_DOWN, NULL, 0));
/* CHECK_RETURN(...) removed due to a bug:
* If `whd_wifi_set_ioctl_buffer` fails, then `whd_thread_quit` is not called,
* `whd_driver` is freed while used by `whd_thread_func`, what causes SEGFAULT.
*/
whd_wifi_set_ioctl_buffer(ifp, WLC_DOWN, NULL, 0);
whd_driver->internal_info.whd_wlan_status.state = WLAN_DOWN;

/* Disable SDIO/SPI interrupt */
Expand Down
Loading