[PATCH] broadband-modem-qmi: fix potential use-after-freed issues

[Dan Williams]

On Thu, 2017-08-03 at 13:25 -0700, Ben Chan wrote:
> This patch fixes some potential use-after-freed issues in
> dms_get_ids_ready(). When an invalid ESN / MEID is retrieved,
> `ctx->self->priv->esn' / `ctx->self->priv->meid' is freed but not
> reset
> to NULL. If no IMEI is retrieved, `str' can be set to the already
> freed
> `ctx->self->priv->esn' / `ctx->self->priv->meid' and then propagated
> to
> a GSimpleAsyncResult object.
> ---
>  src/mm-broadband-modem-qmi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mm-broadband-modem-qmi.c b/src/mm-broadband-modem-
> qmi.c
> index 38356426..bd4d2b6c 100644
> --- a/src/mm-broadband-modem-qmi.c
> +++ b/src/mm-broadband-modem-qmi.c
> @@ -1237,8 +1237,10 @@ dms_get_ids_ready (QmiClientDms *client,
>              ctx->self->priv->esn = g_strdup_printf ("0%s", str);  /*
> zero-pad to 8 chars */
>          else if (len == 8)
>              ctx->self->priv->esn = g_strdup (str);
> -        else
> +        else {
>              mm_dbg ("Invalid ESN reported: '%s' (unexpected
> length)", str);
> +            ctx->self->priv->esn = NULL;

i'm tempted to say that just above here we should just do:

g_clear_pointer (&ctx->self->priv->esn, g_free);

and the same for MEID.  Then we don't forget some time later if we add
more error paths or something.  I'm a big fan of free-and-clear in one
place.  We could also make a macro/inline for this if we want, like
"mm_clear_pointer()" that just does the above.

Dan

> +        }
>      }
>  
>      if (qmi_message_dms_get_ids_output_get_meid (output, &str, NULL)
> &&
> @@ -1247,8 +1249,10 @@ dms_get_ids_ready (QmiClientDms *client,
>          len = strlen (str);
>          if (len == 14)
>              ctx->self->priv->meid = g_strdup (str);
> -        else
> +        else {
>              mm_dbg ("Invalid MEID reported: '%s' (unexpected
> length)", str);
> +            ctx->self->priv->meid = NULL;
> +        }
>      }
>  
>      if (ctx->self->priv->imei)