[PATCH 1/2] qmi-firmware-update: fix seg fault in case of error in hdlc_unframe

[Aleksander Morgado]

On Tue, Jan 17, 2017 at 9:17 AM, Christophe Ronco <c.ronco at kerlink.fr> wrote:
>  src/qmi-firmware-update/qfu-qdl-device.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/qmi-firmware-update/qfu-qdl-device.c b/src/qmi-firmware-update/qfu-qdl-device.c
> index d045ad9..d5252c1 100644
> --- a/src/qmi-firmware-update/qfu-qdl-device.c
> +++ b/src/qmi-firmware-update/qfu-qdl-device.c
> @@ -413,7 +413,7 @@ receive_response (QfuQdlDevice  *self,
>      unframed_size = hdlc_unframe (self->priv->buffer->data, frame_size, self->priv->secondary_buffer->data, self->priv->secondary_buffer->len, error);
>      if (unframed_size == 0) {
>          g_prefix_error (error, "error unframing message: ");
> -        return FALSE;
> +        return -1;
>      }
>

I agree on this one, this is clearly a bug.

>      /* Debug output */
> @@ -490,7 +490,7 @@ qfu_qdl_device_ufopen (QfuQdlDevice  *self,
>          return FALSE;
>
>      rsplen = send_receive (self, self->priv->buffer->data, reqlen, TRUE, 1, &rsp, cancellable, error);
> -    if (rsplen < 0)
> +    if (rsplen <= 0)
>          return FALSE;
>
>      switch (rsp[0]) {

But I don't agree on this one, basically because rsplen should never
be 0. I'm guessing you had the segfault because FALSE was being
returned (0) and that would break reading rsp[0] as rsplen was not
negative. With the FALSE/-1 fix, that should never happen again.

Will import the first part of the patch in a commit.

-- 
Aleksander
https://aleksander.es