Re: [PATCH] bearer-mbim: avoid accessing invalid session_id and nw_error

2017-08-04 Thread Aleksander Morgado
On 03/08/17 22:30, Dan Williams wrote:
> On Thu, 2017-08-03 at 13:24 -0700, Ben Chan wrote:
>> This patch fixes an issue in disconnect_set_ready(). If
>> mbim_message_connect_response_parse(), `session_id' and `nw_error'
>> are
>> not set to a valid value, and thus shouldn't be used.
> 
> LGTM; kinda convoluted function, but I can't think of a better way to
> do it.
> 

Yes, it really is a bit convoluted; I'll suggest an update in a follow up email.
For now, pushed this one, plus an additional commit changing the log messages a 
bit.

> 
>> ---
>>  src/mm-bearer-mbim.c | 8 ++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mm-bearer-mbim.c b/src/mm-bearer-mbim.c
>> index 63bb0579..b6f4bd72 100644
>> --- a/src/mm-bearer-mbim.c
>> +++ b/src/mm-bearer-mbim.c
>> @@ -1119,11 +1119,15 @@ disconnect_set_ready (MbimDevice *device,
>>  } else if (g_error_matches (error,
>>  MBIM_STATUS_ERROR,
>>  MBIM_STATUS_ERROR_CONTEXT_NOT_AC
>> TIVATED)) {
>> -mm_dbg ("Session ID '%u' already disconnected.",
>> session_id);
>> +if (parsed_result)
>> +mm_dbg ("Session ID '%u' already disconnected.",
>> session_id);
>> +else
>> +mm_dbg ("Session ID '' already
>> disconnected.");
>> +
>>  g_clear_error ();
>>  g_clear_error (_error);
>>  } else if (g_error_matches (error, MBIM_STATUS_ERROR,
>> MBIM_STATUS_ERROR_FAILURE)) {
>> -if (nw_error) {
>> +if (parsed_result && nw_error != 0) {
>>  g_error_free (error);
>>  error = mm_mobile_equipment_error_from_mbim_nw_error
>> (nw_error);
>>  }


-- 
Aleksander
https://aleksander.es
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


Re: [PATCH] bearer-mbim: avoid accessing invalid session_id and nw_error

2017-08-03 Thread Dan Williams
On Thu, 2017-08-03 at 13:24 -0700, Ben Chan wrote:
> This patch fixes an issue in disconnect_set_ready(). If
> mbim_message_connect_response_parse(), `session_id' and `nw_error'
> are
> not set to a valid value, and thus shouldn't be used.

LGTM; kinda convoluted function, but I can't think of a better way to
do it.

Dan

> ---
>  src/mm-bearer-mbim.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mm-bearer-mbim.c b/src/mm-bearer-mbim.c
> index 63bb0579..b6f4bd72 100644
> --- a/src/mm-bearer-mbim.c
> +++ b/src/mm-bearer-mbim.c
> @@ -1119,11 +1119,15 @@ disconnect_set_ready (MbimDevice *device,
>  } else if (g_error_matches (error,
>  MBIM_STATUS_ERROR,
>  MBIM_STATUS_ERROR_CONTEXT_NOT_AC
> TIVATED)) {
> -mm_dbg ("Session ID '%u' already disconnected.",
> session_id);
> +if (parsed_result)
> +mm_dbg ("Session ID '%u' already disconnected.",
> session_id);
> +else
> +mm_dbg ("Session ID '' already
> disconnected.");
> +
>  g_clear_error ();
>  g_clear_error (_error);
>  } else if (g_error_matches (error, MBIM_STATUS_ERROR,
> MBIM_STATUS_ERROR_FAILURE)) {
> -if (nw_error) {
> +if (parsed_result && nw_error != 0) {
>  g_error_free (error);
>  error = mm_mobile_equipment_error_from_mbim_nw_error
> (nw_error);
>  }
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel