[PATCH 0/2] Fix a regulatory domain use-after-free on device power-on.

[Grant Erickson <gerickson@nuovations.com>]

Since fe9307c987a0 ("device: Setup regdom when powering up to maintain
consistency"), powering on a Wi-Fi device reapplies the technology's
stored regulatory domain by passing 'technology->regdom' itself down to
wpa_supplicant. gsupplicant keeps that pointer, without copying it,
until the asynchronous 'Country' property set completes, and then hands
it back to the callback, which reaches 'connman_technology_regdom_notify'.
That function frees 'technology->regdom' and then duplicates 'alpha2'
from the string it has just freed.

On a 32-bit ARM device whose regulatory domain is set from the cellular
network by the oFono plugin, connmand built with AddressSanitizer aborts
on every Wi-Fi 'connmanctl disable' followed by 'connmanctl enable':

    ERROR: AddressSanitizer: heap-use-after-free on address 0x75324210
    READ of size 3 at 0x75324210 thread T0
    0x75324210 is located 0 bytes inside of 3-byte region
    freed by thread T0 here:
        #1 connman_technology_regdom_notify src/technology.c:299
        #2 __connman_technology_notify_regdom_by_device src/technology.c:1150
        #3 connman_device_regdom_notify src/device.c:1101
        #4 wifi_regdom_callback plugins/wifi.c:2060
        #5 country_result gsupplicant/supplicant.c:3715
        #6 property_set_reply gsupplicant/dbus.c:359

The read, from gdb at the abort, is the 'g_strdup' at
src/technology.c:300, in the same call. The request that carried the
pointer came from 'connman_device_set_powered', by way of
'connman_device_set_regdom', 'wifi_set_regdom' and
'g_supplicant_interface_set_country'.

Patch 1 is the fix: gsupplicant owns a copy of 'alpha2' for the life of
each 'Country' set request. That also covers two related cases in which
the string is freed by a call other than the one that reads it: a new
regulatory domain arriving, through 'connman_technology_set_regdom',
while a power-on request is outstanding; and the timezone module, which
frees its 'alpha2' right after passing it to
'connman_technology_set_regdom'.

Patch 2 makes 'connman_technology_regdom_notify' safe when 'alpha2' is
the technology's own 'regdom'. On its own, it stops the crash above but
not those two cases, so it is defense-in-depth rather than an
alternative to patch 1.

Grant Erickson (2):
  gsupplicant: Own a copy of 'alpha2' for the life of a 'Country' set.
  technology: Make 'connman_technology_regdom_notify' alias-safe.

 gsupplicant/supplicant.c | 9 ++++++---
 src/technology.c         | 9 ++++++++-
 2 files changed, 14 insertions(+), 4 deletions(-)

-- 
2.52.0