From 020721de929fcea7e147c21935bd708e3bd6de26 Mon Sep 17 00:00:00 2001 From: Winford Date: Sun, 25 Feb 2024 17:05:21 -0800 Subject: [PATCH 1/2] Fix ESP32 GPIO `pull` direction validation Any atom was being accepted as a valid pull mode, without error. This could result in `floating` mode being silently set when an invalid atom is given. Invalid modes now return an error to the user. Fixes `pull` mode problem in issue #1071 Signed-off-by: Winford --- CHANGELOG.md | 2 ++ .../esp32/components/avm_builtins/gpio_driver.c | 12 ++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f69e307510..00174fa868 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `binary_to_term` checks atom encoding validity, and fix latin1 support (when non-ASCII chars are used) - ESP32: fixed bug in `gpio:set_pin_mode/2` and `gpio:set_direction/3` that would accept any atom for the mode parameter without an error. +- ESP32: GPIO driver fix bug that would accept invalid `pull` direction, and silently set `pull` direction to `floating` without issuing an error. + ### Changed diff --git a/src/platforms/esp32/components/avm_builtins/gpio_driver.c b/src/platforms/esp32/components/avm_builtins/gpio_driver.c index b300ce07b1..348c0e842b 100644 --- a/src/platforms/esp32/components/avm_builtins/gpio_driver.c +++ b/src/platforms/esp32/components/avm_builtins/gpio_driver.c @@ -82,7 +82,7 @@ static const AtomStringIntPair pull_mode_table[] = { { ATOM_STR("\x4", "down"), GPIO_PULLDOWN_ONLY }, { ATOM_STR("\x7", "up_down"), GPIO_PULLUP_PULLDOWN }, { ATOM_STR("\x8", "floating"), GPIO_FLOATING }, - SELECT_INT_DEFAULT(GPIO_FLOATING) + SELECT_INT_DEFAULT(-1) }; enum gpio_pin_level @@ -153,7 +153,7 @@ static inline term gpio_set_pin_mode(Context *ctx, term gpio_num_term, term mode return OK_ATOM; } -static gpio_pull_mode_t get_pull_mode(Context *ctx, term pull) +static avm_int_t get_pull_mode(Context *ctx, term pull) { return interop_atom_term_select_int(pull_mode_table, pull, ctx->global); } @@ -161,8 +161,12 @@ static gpio_pull_mode_t get_pull_mode(Context *ctx, term pull) static inline term set_pin_pull_mode(Context *ctx, term gpio_num_term, term pull) { avm_int_t gpio_num = term_to_int(gpio_num_term); - gpio_pull_mode_t pull_mode = get_pull_mode(ctx, pull); - esp_err_t result = gpio_set_pull_mode(gpio_num, pull_mode); + avm_int_t pull_mode = get_pull_mode(ctx, pull); + if (UNLIKELY(pull_mode < 0)) { + return ERROR_ATOM; + } + + esp_err_t result = gpio_set_pull_mode(gpio_num, (gpio_pull_mode_t) pull_mode); if (UNLIKELY(result != ESP_OK)) { return ERROR_ATOM; } From cd130e1cb7435367fc0be9e9a8a5c9055862bd32 Mon Sep 17 00:00:00 2001 From: Winford Date: Sun, 25 Feb 2024 16:59:55 -0800 Subject: [PATCH 2/2] Add missing input validation to ESP32 GPIO driver pins Pins would accept any integer, including negative numbers, and invalid pin numbers (example: `128`), possibly leading to a crash of the VM. Pins numbers are now checked, and only valid GPIO pin numbers are accepted. Fixes pin number validation problem in issue #1071 Signed-off-by: Winford --- CHANGELOG.md | 2 +- .../components/avm_builtins/gpio_driver.c | 95 +++++++++++++++++-- 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00174fa868..add7ca530e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 used) - ESP32: fixed bug in `gpio:set_pin_mode/2` and `gpio:set_direction/3` that would accept any atom for the mode parameter without an error. - ESP32: GPIO driver fix bug that would accept invalid `pull` direction, and silently set `pull` direction to `floating` without issuing an error. - +- ESP32: fixed bug in gpio driver that would accept invalid pin numbers (either negative, or too large) ### Changed diff --git a/src/platforms/esp32/components/avm_builtins/gpio_driver.c b/src/platforms/esp32/components/avm_builtins/gpio_driver.c index 348c0e842b..2954ddab7c 100644 --- a/src/platforms/esp32/components/avm_builtins/gpio_driver.c +++ b/src/platforms/esp32/components/avm_builtins/gpio_driver.c @@ -138,7 +138,16 @@ struct GPIOData static inline term gpio_set_pin_mode(Context *ctx, term gpio_num_term, term mode_term) { - int gpio_num = term_to_int(gpio_num_term); + gpio_num_t gpio_num; + if (LIKELY(term_is_integer(gpio_num_term))) { + avm_int_t pin_int = term_to_int32(gpio_num_term); + if (UNLIKELY((pin_int < 0) || (pin_int >= GPIO_NUM_MAX))) { + return ERROR_ATOM; + } + gpio_num = (gpio_num_t) pin_int; + } else { + return ERROR_ATOM; + } avm_int_t mode = interop_atom_term_select_int(pin_mode_table, mode_term, ctx->global); if (UNLIKELY(mode < 0)) { @@ -160,7 +169,17 @@ static avm_int_t get_pull_mode(Context *ctx, term pull) static inline term set_pin_pull_mode(Context *ctx, term gpio_num_term, term pull) { - avm_int_t gpio_num = term_to_int(gpio_num_term); + gpio_num_t gpio_num; + if (LIKELY(term_is_integer(gpio_num_term))) { + avm_int_t pin_int = term_to_int32(gpio_num_term); + if (UNLIKELY((pin_int < 0) || (pin_int >= GPIO_NUM_MAX))) { + return ERROR_ATOM; + } + gpio_num = (gpio_num_t) pin_int; + } else { + return ERROR_ATOM; + } + avm_int_t pull_mode = get_pull_mode(ctx, pull); if (UNLIKELY(pull_mode < 0)) { return ERROR_ATOM; @@ -175,7 +194,17 @@ static inline term set_pin_pull_mode(Context *ctx, term gpio_num_term, term pull static inline term hold_en(term gpio_num_term) { - int gpio_num = term_to_int(gpio_num_term); + gpio_num_t gpio_num; + if (LIKELY(term_is_integer(gpio_num_term))) { + avm_int_t pin_int = term_to_int32(gpio_num_term); + if (UNLIKELY((pin_int < 0) || (pin_int >= GPIO_NUM_MAX))) { + return ERROR_ATOM; + } + gpio_num = (gpio_num_t) pin_int; + } else { + return ERROR_ATOM; + } + esp_err_t result = gpio_hold_en(gpio_num); if (UNLIKELY(result != ESP_OK)) { return ERROR_ATOM; @@ -185,7 +214,17 @@ static inline term hold_en(term gpio_num_term) static inline term hold_dis(term gpio_num_term) { - int gpio_num = term_to_int(gpio_num_term); + gpio_num_t gpio_num; + if (LIKELY(term_is_integer(gpio_num_term))) { + avm_int_t pin_int = term_to_int32(gpio_num_term); + if (UNLIKELY((pin_int < 0) || (pin_int >= GPIO_NUM_MAX))) { + return ERROR_ATOM; + } + gpio_num = (gpio_num_t) pin_int; + } else { + return ERROR_ATOM; + } + esp_err_t result = gpio_hold_dis(gpio_num); if (UNLIKELY(result != ESP_OK)) { return ERROR_ATOM; @@ -195,7 +234,16 @@ static inline term hold_dis(term gpio_num_term) static inline term gpio_digital_write(Context *ctx, term gpio_num_term, term level_term) { - int gpio_num = term_to_int(gpio_num_term); + gpio_num_t gpio_num; + if (LIKELY(term_is_integer(gpio_num_term))) { + avm_int_t pin_int = term_to_int32(gpio_num_term); + if (UNLIKELY((pin_int < 0) || (pin_int >= GPIO_NUM_MAX))) { + return ERROR_ATOM; + } + gpio_num = (gpio_num_t) pin_int; + } else { + return ERROR_ATOM; + } int level; if (term_is_integer(level_term)) { @@ -220,7 +268,17 @@ static inline term gpio_digital_write(Context *ctx, term gpio_num_term, term lev static inline term gpio_digital_read(term gpio_num_term) { - avm_int_t gpio_num = term_to_int(gpio_num_term); + gpio_num_t gpio_num; + if (LIKELY(term_is_integer(gpio_num_term))) { + avm_int_t pin_int = term_to_int32(gpio_num_term); + if (UNLIKELY((pin_int < 0) || (pin_int >= GPIO_NUM_MAX))) { + return ERROR_ATOM; + } + gpio_num = (gpio_num_t) pin_int; + } else { + return ERROR_ATOM; + } + avm_int_t level = gpio_get_level(gpio_num); return level ? HIGH_ATOM : LOW_ATOM; @@ -353,7 +411,18 @@ static term gpiodriver_set_int(Context *ctx, int32_t target_pid, term cmd) struct GPIOData *gpio_data = ctx->platform_data; - int32_t gpio_num = term_to_int32(term_get_tuple_element(cmd, 1)); + term gpio_num_term = term_to_int32(term_get_tuple_element(cmd, 1)); + gpio_num_t gpio_num; + if (LIKELY(term_is_integer(gpio_num_term))) { + avm_int_t pin_int = term_to_int32(gpio_num_term); + if (UNLIKELY((pin_int < 0) || (pin_int >= GPIO_NUM_MAX))) { + return ERROR_ATOM; + } + gpio_num = (gpio_num_t) pin_int; + } else { + return ERROR_ATOM; + } + term trigger = term_get_tuple_element(cmd, 2); if (term_get_tuple_arity(cmd) == 4) { term pid = term_get_tuple_element(cmd, 3); @@ -445,7 +514,17 @@ static term gpiodriver_remove_int(Context *ctx, term cmd) { struct GPIOData *gpio_data = ctx->platform_data; - int32_t gpio_num = term_to_int32(term_get_tuple_element(cmd, 1)); + term gpio_num_term = term_to_int32(term_get_tuple_element(cmd, 1)); + gpio_num_t gpio_num; + if (LIKELY(term_is_integer(gpio_num_term))) { + avm_int_t pin_int = term_to_int32(gpio_num_term); + if (UNLIKELY((pin_int < 0) || (pin_int >= GPIO_NUM_MAX))) { + return ERROR_ATOM; + } + gpio_num = (gpio_num_t) pin_int; + } else { + return ERROR_ATOM; + } struct ListHead *item; struct ListHead *tmp;