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

[João Paulo Rechi Vita]

On Wed, Nov 20, 2013 at 12:01 PM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> 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?
>

I like the alternative you have proposed, I'll send an updated version
implementing it.

-- 
João Paulo Rechi Vita
http://about.me/jprvita