From 6efbb1a9e4b91b60fb955bdfd904be5cfb659168 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Wed, 3 Jan 2024 20:59:14 +0100 Subject: [PATCH] fix(modem): Fixed AT commands to copy strings to prevent overrides Previously we used std::string_view, which pointed to the lower-layers buffer which might have been reused for other asynchronous operations before processing it, thus causing corruption of replies. Closes https://github.com/espressif/esp-protocols/issues/463 --- .../esp_modem_command_library_utils.hpp | 8 +-- .../src/esp_modem_command_library.cpp | 62 ++++++++++++------- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/components/esp_modem/include/cxx_include/esp_modem_command_library_utils.hpp b/components/esp_modem/include/cxx_include/esp_modem_command_library_utils.hpp index b47c150d88c..9cae9cdf9b6 100644 --- a/components/esp_modem/include/cxx_include/esp_modem_command_library_utils.hpp +++ b/components/esp_modem/include/cxx_include/esp_modem_command_library_utils.hpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -25,18 +25,16 @@ command_result generic_command(CommandableIf *t, const std::string &command, * @brief Utility command to send command and return reply (after DCE says OK) * @param t Anything that is "command-able" * @param command Command to issue - * @param output String to return - * @param timeout_ms + * @param output String to return (could be either std::string& or std::string_view&) * @param timeout_ms Command timeout in ms * @return Generic command return type (OK, FAIL, TIMEOUT) */ -command_result generic_get_string(CommandableIf *t, const std::string &command, std::string &output, uint32_t timeout_ms = 500); +template command_result generic_get_string(CommandableIf *t, const std::string &command, T &output, uint32_t timeout_ms = 500); /** * @brief Generic command that passes on "OK" and fails on "ERROR" * @param t Anything that is "command-able" * @param command Command to issue - * @param timeout * @param timeout_ms Command timeout in ms * @return Generic command return type (OK, FAIL, TIMEOUT) */ diff --git a/components/esp_modem/src/esp_modem_command_library.cpp b/components/esp_modem/src/esp_modem_command_library.cpp index 976869e5138..d356205ab0f 100644 --- a/components/esp_modem/src/esp_modem_command_library.cpp +++ b/components/esp_modem/src/esp_modem_command_library.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -51,7 +51,31 @@ command_result generic_command(CommandableIf *t, const std::string &command, return generic_command(t, command, pass, fail, timeout_ms); } -static command_result generic_get_string(CommandableIf *t, const std::string &command, std::string_view &output, uint32_t timeout_ms = 500) +namespace str_copy { + +bool set(std::string &dest, std::string_view &src) +{ + dest = src; + return true; +} + +/* + * Purpose of this definition is to support string_view output in generic_get_string() + */ +bool set(std::string_view &dest, std::string_view &src) +{ + if (dest.size() >= src.size()) { + std::copy(src.begin(), src.end(), (char *)dest.data()); + dest = dest.substr(0, src.size()); + return true; + } + ESP_LOGE(TAG, "Cannot set string_view of size %d (need %d)", dest.size(), src.size()); + return false; +} + +} // str_copy + +template command_result generic_get_string(CommandableIf *t, const std::string &command, T &output, uint32_t timeout_ms) { ESP_LOGV(TAG, "%s", __func__ ); return t->command(command, [&](uint8_t *data, size_t len) { @@ -70,7 +94,9 @@ static command_result generic_get_string(CommandableIf *t, const std::string &co } else if (token.find("ERROR") != std::string::npos) { return command_result::FAIL; } else if (token.size() > 2) { - output = token; + if (!str_copy::set(output, token)) { + return command_result::FAIL; + } } response = response.substr(pos + 1); } @@ -78,18 +104,6 @@ static command_result generic_get_string(CommandableIf *t, const std::string &co }, timeout_ms); } -command_result generic_get_string(CommandableIf *t, const std::string &command, std::string &output, uint32_t timeout_ms) -{ - ESP_LOGV(TAG, "%s", __func__ ); - std::string_view out; - auto ret = generic_get_string(t, command, out, timeout_ms); - if (ret == command_result::OK) { - output = out; - } - return ret; -} - - command_result generic_command_common(CommandableIf *t, const std::string &command, uint32_t timeout_ms) { ESP_LOGV(TAG, "%s", __func__ ); @@ -153,7 +167,7 @@ command_result hang_up(CommandableIf *t) command_result get_battery_status(CommandableIf *t, int &voltage, int &bcs, int &bcl) { ESP_LOGV(TAG, "%s", __func__ ); - std::string_view out; + std::string out; auto ret = generic_get_string(t, "AT+CBC\r", out); if (ret != command_result::OK) { return ret; @@ -189,7 +203,7 @@ command_result get_battery_status(CommandableIf *t, int &voltage, int &bcs, int command_result get_battery_status_sim7xxx(CommandableIf *t, int &voltage, int &bcs, int &bcl) { ESP_LOGV(TAG, "%s", __func__ ); - std::string_view out; + std::string out; auto ret = generic_get_string(t, "AT+CBC\r", out); if (ret != command_result::OK) { return ret; @@ -224,7 +238,7 @@ command_result set_flow_control(CommandableIf *t, int dce_flow, int dte_flow) command_result get_operator_name(CommandableIf *t, std::string &operator_name, int &act) { ESP_LOGV(TAG, "%s", __func__ ); - std::string_view out; + std::string out; auto ret = generic_get_string(t, "AT+COPS?\r", out, 75000); if (ret != command_result::OK) { return ret; @@ -361,7 +375,7 @@ command_result set_cmux(CommandableIf *t) command_result read_pin(CommandableIf *t, bool &pin_ok) { ESP_LOGV(TAG, "%s", __func__ ); - std::string_view out; + std::string out; auto ret = generic_get_string(t, "AT+CPIN?\r", out); if (ret != command_result::OK) { return ret; @@ -397,7 +411,7 @@ command_result at(CommandableIf *t, const std::string &cmd, std::string &out, in command_result get_signal_quality(CommandableIf *t, int &rssi, int &ber) { ESP_LOGV(TAG, "%s", __func__ ); - std::string_view out; + std::string out; auto ret = generic_get_string(t, "AT+CSQ\r", out); if (ret != command_result::OK) { return ret; @@ -435,7 +449,7 @@ command_result set_network_attachment_state(CommandableIf *t, int state) command_result get_network_attachment_state(CommandableIf *t, int &state) { ESP_LOGV(TAG, "%s", __func__ ); - std::string_view out; + std::string out; auto ret = generic_get_string(t, "AT+CGATT?\r", out); if (ret != command_result::OK) { return ret; @@ -462,7 +476,7 @@ command_result set_radio_state(CommandableIf *t, int state) command_result get_radio_state(CommandableIf *t, int &state) { ESP_LOGV(TAG, "%s", __func__ ); - std::string_view out; + std::string out; auto ret = generic_get_string(t, "AT+CFUN?\r", out); if (ret != command_result::OK) { return ret; @@ -527,7 +541,7 @@ command_result set_network_bands_sim76xx(CommandableIf *t, const std::string &mo command_result get_network_system_mode(CommandableIf *t, int &mode) { ESP_LOGV(TAG, "%s", __func__ ); - std::string_view out; + std::string out; auto ret = generic_get_string(t, "AT+CNSMOD?\r", out); if (ret != command_result::OK) { return ret; @@ -555,7 +569,7 @@ command_result set_gnss_power_mode(CommandableIf *t, int mode) command_result get_gnss_power_mode(CommandableIf *t, int &mode) { ESP_LOGV(TAG, "%s", __func__ ); - std::string_view out; + std::string out; auto ret = generic_get_string(t, "AT+CGNSPWR?\r", out); if (ret != command_result::OK) { return ret;