[pulseaudio-discuss] [PATCH] bluetooth: Fix crash in pa_bluetooth_discovery_get_device_by_address()

Tanu Kaskinen tanu.kaskinen at linux.intel.com
Wed Nov 20 07:01:24 PST 2013


On Wed, 2013-11-20 at 10:52 -0300, jprvita at gmail.com wrote:
> From: João Paulo Rechi Vita <jprvita at gmail.com>
> 
> We need to check if d->adapter is valid before dereferencing it, and
> also make sure both address strings are valid before calling pa_streq().
> ---
>  src/modules/bluetooth/bluez5-util.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index c8ff219..53330ab 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -395,7 +395,9 @@ pa_bluetooth_device* pa_bluetooth_discovery_get_device_by_address(pa_bluetooth_d
>      pa_assert(local);
>  
>      while ((d = pa_hashmap_iterate(y->devices, &state, NULL)))
> -        if (pa_streq(d->address, remote) && pa_streq(d->adapter->address, local))
> +        if (d->address && pa_streq(d->address, remote) &&
> +            d->adapter && d->adapter->address && pa_streq(d->adapter->address, local))
> +
>              return d->device_info_valid == 1 ? d : NULL;
>  
>      return NULL;

I don't like all this validity checking, when all that should be needed
is to check whether device_info_valid is 1.

I propose that d->device_info_valid is checked before checking anything
else. That's not sufficient, however - we also need to set
device_info_valid to -1 if the adapter is invalid (doesn't have an
address). This can be done at the end of
parse_interfaces_and_properties(), as an amendment to this code:

        if (!d->adapter && d->adapter_path) {
            d->adapter = pa_hashmap_get(d->discovery->adapters, d->adapter_path);
            if (!d->adapter) {
                pa_log_error("Device %s is child of nonexistent adapter %s", d->path, d->adapter_path);
                set_device_info_valid(d, -1);
            } else
                set_device_info_valid(d, 1);
        }

What do you think?

-- 
Tanu



More information about the pulseaudio-discuss mailing list