nrf5x: array overflow in gpio configuration code

[andrewleech]

Description

• Type: Bug
• Related PR: NRF5x: Don't allocate GPIOTE in DigitalIn #5207
• Priority: Blocker

---

Bug

Target
nrf5x

Toolchain:
GCC_ARM|ARM|IAR

Toolchain version:
Tested on Windows, GNU Tools ARM Embedded\7 2017-q4-major

Steps to reproduce
The PR I've marked as related, #5207, was a good change in that it stopped valuable gpiote resources being used for a basic input pin. However this has unearthed / created an array overflow elsewhere in the gpio init.

The problem starts with gpio_dir() and gpio_mode() functions in mbed-os/targets/TARGET_NORDIC/TARGET_NRF5/gpio_api.c.

Both of these functions' implementations call gpiote_pin_uninit() then gpio_apply_config() with appropriate pin configs.

Now gpiote_pin_uninit() (also in gpio_api.c) only checks if the pin has been configured at all before running the appropriate nordic nrf_drv_gpiote_*_uninit() function.

The problem is nrf_drv_gpiote_in_uninit() queries which gpiote channel is in use for the provided pin then frees that channel by writing 0xFF to it's internal tracking array (m_cb.handlers) at the channel index.
If the pin is not currently in use by gpiote however the channel returned is -1 so we get m_cb.handlers[-1] = 0xFF being run.

With the recent change to not use (waste) a gpiote for the input pin, now this function is being run without previously having gpiote channel set so writes a 0xFF to some random part of ram.

For some unfathomable reason the nrf function to query whether a pin is currently assigned to a channel is internal-only, so we can't check if a gpiote is in use before running nrf_drv_gpiote_in_uninit(). I think we either have to:

• track pin / channel usage ourself, basically duplicating the internal nrf functionality
  or
• change to an ask for forgiveness model in gpio_mode() and gpio_dir(), run nrf_drv_gpiote_in_init() immediately and if it fails due to NRF_ERROR_INVALID_STATE (returned when gpiote already configured) then run the nrf_drv_gpiote_in_uninit() function and re-run nrf_drv_gpiote_in_init()

The ask for forgiveness is probably more efficient, with less duplicated functionality but does leave nrf_drv_gpiote_in_uninit() as a potentially unsafe function to call so is probably not the best route.

The other route of tracking gpiote usage separate to generic gpio config is annoying but safer, I'll attempt an implementation of this now.

On a related note, the nordic internal function for checking if a pin is currently assigned to a gpiote channel; pin_in_use_by_gpiote(). It's a static inline however so we can't get access to it.
nrf_drv_gpiote_in_uninit() does ASSERT using this function to ensure a pin is in use, but nordic's ASSERT does not seem to be enabled at all.
This should probably be wired through to mbed_error() when in debug mode, I'll look at implementing this separate to fixing this issue as it would have caught the issue much earlier.

[andrewleech]

Turns out the fix was much easier than expected, we basically track the required information already.
Testing alongside my enabled asserts (#6022) shows it to work well.

[0xc0170]

@ARMmbed/team-nordic please review

[ciarmcom]

ARM Internal Ref: MBOTRIAGE-271

[andrewleech]

Sorry, this issue should have been closed by #6021