Re: [PATCH 1/2] scan: check scan request in get_survey_done before deref

[James Prestwood <prestwoj@gmail.com>]

Hi Denis,

On 9/5/24 7:37 AM, Denis Kenzior wrote:
> Hi James,
>
> On 9/5/24 8:43 AM, James Prestwood wrote:
>> Due to the possibility of external scans the scan request pointer
>
> Do we know why external scans are happening?  I can understand a one 
> off because someone triggered iw scan, but from the bug report it 
> sounded like it was crashing iwd repeatedly?
NetworkManager? wpa_supplicant running? I really have no idea. Either 
way its out of our control.
>
>> could be NULL. Prior to surveys IWD would still get the results in
>> order for periodic scans to utilize them. This behavior can be
>> retained by checking both if we don't have a request or if the
>> request was canceled. This check is identical to the one in
>> get_scan_done.
>>
>> This fixes a crash when checking if the NULL scan request has been
>> canceled:
>>
>> 0x00005ffa6a0376de in get_survey_done (user_data=0x5ffa783a3f90) at 
>> src/scan.c:2059
>> 0x0000749646a29bbd in ?? () from /usr/lib/libell.so.0
>> 0x0000749646a243cb in ?? () from /usr/lib/libell.so.0
>> 0x0000749646a24655 in l_main_iterate () from /usr/lib/libell.so.0
>> 0x0000749646a24ace in l_main_run () from /usr/lib/libell.so.0
>> 0x0000749646a263a4 in l_main_run_with_signal () from 
>> /usr/lib/libell.so.0
>> 0x00005ffa6a00d642 in main (argc=<optimized out>, argv=<optimized 
>> out>) at src/main.c:614
>>
>> Reported-by: Daniel Bond <danielbondno@gmail.com>
>> ---
>>   src/scan.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/scan.c b/src/scan.c
>> index debdeb1f..205365cd 100644
>> --- a/src/scan.c
>> +++ b/src/scan.c
>> @@ -2056,7 +2056,7 @@ static void get_survey_done(void *user_data)
>>         sc->get_survey_cmd_id = 0;
>>   -    if (!results->sr->canceled)
>> +    if (!results->sr || !results->sr->canceled)
>
> I still don't understand why we're even bothering requesting a survey 
> for a scan we didn't trigger?  In other words, we shouldn't even be in 
> this function.
Because we still utilize external scan results for periodic scans. So if 
periodic scans are running we will still try and get the survey/results.
>
>>           get_results(results);
>>       else
>>           get_scan_done(user_data);
>
> Regards,
> -Denis