From b9db53595056efe0d0cd012913245cd066eb799f Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Sun, 3 Oct 2021 19:56:05 +0300 Subject: [PATCH] light: fix out-of-bounds access when using GPIO16 As noticed in the #2472 Internal implemementation still lacks the support for the GPIO16, as it needs to use 'special' IO16 registers (and due to the fact that the 'normal' registers only fit in an u16[16], from 0 to 15, so internals need to change as well) One possible way is to attach certain implementation funcs to the struct handling the isr, avoiding a bunch of in-line checks for `pin == 16` Another option is to just use Core's `analogWrite`, which hides the implementation from us and should work pretty seamlessly. (...but, currently has 2 different waveform generator types, and it is not really clear which one is a better default, as it needs to be set at build-time) --- code/espurna/compat.h | 5 +++++ code/espurna/light.cpp | 14 +++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/code/espurna/compat.h b/code/espurna/compat.h index 8a7d9d9e..d311c837 100644 --- a/code/espurna/compat.h +++ b/code/espurna/compat.h @@ -121,6 +121,11 @@ constexpr const T& clamp(const T& value, const T& low, const T& high) { return (value < low) ? low : (high < value) ? high : value; } +template +constexpr size_t size(const T (&)[Size]) { + return Size; +} + } // namespace std #endif diff --git a/code/espurna/light.cpp b/code/espurna/light.cpp index 6ce31d43..93a25055 100644 --- a/code/espurna/light.cpp +++ b/code/espurna/light.cpp @@ -2906,20 +2906,24 @@ void lightTransition(LightTransition transition) { namespace { #if LIGHT_PROVIDER == LIGHT_PROVIDER_DIMMER -const unsigned long _light_iomux[16] PROGMEM = { +const unsigned long _light_iomux[] PROGMEM = { PERIPHS_IO_MUX_GPIO0_U, PERIPHS_IO_MUX_U0TXD_U, PERIPHS_IO_MUX_GPIO2_U, PERIPHS_IO_MUX_U0RXD_U, PERIPHS_IO_MUX_GPIO4_U, PERIPHS_IO_MUX_GPIO5_U, PERIPHS_IO_MUX_SD_CLK_U, PERIPHS_IO_MUX_SD_DATA0_U, PERIPHS_IO_MUX_SD_DATA1_U, PERIPHS_IO_MUX_SD_DATA2_U, PERIPHS_IO_MUX_SD_DATA3_U, PERIPHS_IO_MUX_SD_CMD_U, PERIPHS_IO_MUX_MTDI_U, PERIPHS_IO_MUX_MTCK_U, PERIPHS_IO_MUX_MTMS_U, PERIPHS_IO_MUX_MTDO_U }; -const unsigned long _light_iofunc[16] PROGMEM = { +const unsigned long _light_iofunc[] PROGMEM = { FUNC_GPIO0, FUNC_GPIO1, FUNC_GPIO2, FUNC_GPIO3, FUNC_GPIO4, FUNC_GPIO5, FUNC_GPIO6, FUNC_GPIO7, FUNC_GPIO8, FUNC_GPIO9, FUNC_GPIO10, FUNC_GPIO11, FUNC_GPIO12, FUNC_GPIO13, FUNC_GPIO14, FUNC_GPIO15 }; +bool _lightIoMuxPin(unsigned char pin) { + return (pin < std::size(_light_iofunc)) && (pin < std::size(_light_iomux)); +} + #endif inline bool _lightUseGamma(size_t channels, size_t index) { @@ -3162,14 +3166,14 @@ void lightSetup() { // Load up until first GPIO_NONE. Allow settings to override, but not remove values const auto pin = Light::settings::channelPin(index); - if (!gpioValid(pin)) { + if (!gpioValid(pin) || !_lightIoMuxPin(pin)) { break; } _light_channels.emplace_back(pin); - io_info[index][0] = pgm_read_dword(&_light_iomux[pin]); - io_info[index][1] = pgm_read_dword(&_light_iofunc[pin]); + io_info[index][0] = _light_iomux[pin]; + io_info[index][1] = _light_iofunc[pin]; io_info[index][2] = pin; pinMode(pin, OUTPUT);