[Bug report] nRF SDK GPIOTE bug

[trentrolf]

Bug in the SDK function below. channel_free is called whether or not pin_in_use_by_te() is true or not. For us this resulted in channel_free() getting called with an invalid channel_port of FF which resulted in an out of bounds dereference and memory corruption. I would suggest only calling channel_free() if pin_in_use_by_te() is true.

We found this in v13.0.0, but it looks like it's still in the latest version.

I'm aware that we probably shouldn't be calling uninit on a function that hasn't had init called first, but regardless I think the protection would be wise.

void nrfx_gpiote_in_uninit(nrfx_gpiote_pin_t pin)
{
    NRFX_ASSERT(nrf_gpio_pin_present_check(pin));
    NRFX_ASSERT(pin_in_use_by_gpiote(pin));
    nrfx_gpiote_in_event_disable(pin);
    if (pin_in_use_by_te(pin))
    {
        nrf_gpiote_te_default((uint32_t)channel_port_get(pin));
    }
    if (pin_configured_check(pin))
    {
        nrf_gpio_cfg_default(pin);
        pin_configured_clear(pin);
    }
    channel_free((uint8_t)channel_port_get(pin));
    pin_in_use_clear(pin);
}

[Stian Røed Hafskjold]

Short update on this: This was never fixed because this is not considered to be a bug - if `channel_port_get` returns 0xFF it means that user attempts to un-initialize a pin that was not initialized before. This is not a correct use-case and it is guarded by the assertion: https://github.com/NordicSemiconductor/nrfx/blob/d779b49fc59c7a165e7da1d7cd7d57b28a059f16/drivers/src/nrfx_gpiote.c#L805

[bvyates]

I was chasing this issue for a while as well, and agree with the original poster that nrfx_gpiote_in_uninit could check for this situation and avoid a bad OOB access. If the pin_in_use functions were exposed, that would be a decent resolution as well; I'd be fine checking if it is initialized before calling uninit myself.

My work around for this was to always init the channel with an empty callback function (and ignore errors if it is already initialized) before any call to uninit, so the OOB access is always avoided. This is a bit of a painful hack though, so either being able to check myself if the channel has been initialized or making uninit safe for repeated calls would be much preferred.

Thank you!