Missing NULL checks - qmi-proxy.c

[Peter Naulls]

On 12/21/20 8:09 AM, Aleksander Morgado wrote:
> Hey Peter
> 
>> Here are some trivial null checks I found during testing.  I believe there are
>> some more issues remaining.  This is against both latest release 1.2.26.6 and
>> current git.
>>
> 
> Are any of these new checks added to fix bugs? Or just through code review?
> See comments below.

Bug fixes.   Found with GDB, for example:

Thread 1 "qmi-proxy" received signal SIGSEGV, Segmentation fault.
0x77c93d41 in qmi_client_info_array_lookup_cid (array=0x0, 
service=QMI_SERVICE_LOC, cid=2 '<error reading variable>) at qmi-proxy.c:544
544	qmi-proxy.c: No such file or directory.
(gdb) bt
#0  0x77c93d41 in qmi_client_info_array_lookup_cid (array=0x0, 
service=QMI_SERVICE_LOC, cid=2 '<error reading variable>) at qmi-proxy.c:544
#1  0x77c94581 in track_implicit_cid (self=0x77958a10, client=0x41c678, 
message=0x416a00) at qmi-proxy.c:675
#2  0x77c94e9b in process_message (self=0x77958a10, client=0x41c678, 
message=0x416a00) at qmi-proxy.c:853
#3  0x77c94fad in parse_request (self=0x77958a10, client=0x41c678) at 
qmi-proxy.c:905
#4  0x77c9513f in connection_readable_cb (warning: GDB can't find the start of 
the function at 0x77b5717e.


> 
>> --- a/src/libqmi-glib/qmi-proxy.c       2020-12-17 20:07:45.620241110 -0500
>> +++ b/src/libqmi-glib/qmi-proxy.c       2020-12-17 20:21:32.405975476 -0500
>> @@ -212,7 +212,7 @@
>>    {
>>        guint i;
>>
>> -    if (!client->qmi_client_info_array->len)
>> +    if (!client->qmi_client_info_array || !client->qmi_client_info_array->len)
>>            return;
> 
> The client->qmi_client_info_array is allocated as soon as the Client
> struct is allocated, and the array is removed when the Client struct
> is freed; so there is (should be) no chance that this array is every
> NULL.

Well, no doubt.  Still, this is what I'm seeing.  This code is run
via qmi-proxy.

> 
> If you have a way to reproduce this issue, please run the qmi-proxy
> under valgrind:
> https://gitlab.freedesktop.org/mobile-broadband/libqmi/-/merge_requests/181#note_716877

Unfortunately, valgrind doesn't want to seem to play ball on my ramips system, 
but I'll let you know if I have anything else.

Regards.