Re: [PATCH] cgen: opcodes: Fix memory corruption in in lookup

[Yao Qi <qiyaoltc at gmail dot com>]

Hi Stafford,
opcodes/ changes should be reviewed in binutils@sourceware.org.

On Wed, Feb 8, 2017 at 4:40 PM, Stafford Horne <shorne@gmail.com> wrote:
> The buf variable is used after it is free'd.  This causes the lookups to
> fail and also causes memory corruption.
>
> Re-arrange the code a bit to make sure we always free memory before
> returning. This was caught in openrisc testing, one of the only user of
> this method.
>
> opcodes/ChangeLog:
>
> 2017-02-09  Stafford Horne  <shorne@gmail.com>
>         cgen-opc.c (cgen_lookup_insn): Fix memory corruption issue.

ChangeLog format issue,

017-02-09  Stafford Horne  <shorne@gmail.com>

* cgen-opc.c (cgen_lookup_insn): Fix memory corruption issue.

> ---
>  opcodes/cgen-opc.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/opcodes/cgen-opc.c b/opcodes/cgen-opc.c
> index 72b4f05..40a6320 100644
> --- a/opcodes/cgen-opc.c
> +++ b/opcodes/cgen-opc.c
> @@ -463,7 +463,6 @@ cgen_lookup_insn (CGEN_CPU_DESC cd,
>        buf = (unsigned char *) xmalloc (cd->max_insn_bitsize / 8);
>        cgen_put_insn_value (cd, buf, length, insn_int_value);
>        base_insn = insn_int_value;
> -      free (buf);
>      }
>    else
>      {
> @@ -475,7 +474,7 @@ cgen_lookup_insn (CGEN_CPU_DESC cd,
>        base_insn = cgen_get_insn_value (cd, buf, length);
>      }
>
> -  if (!insn)
> +  if (insn == NULL)
>      {
>        const CGEN_INSN_LIST *insn_list;
>
> @@ -505,7 +504,8 @@ cgen_lookup_insn (CGEN_CPU_DESC cd,
>                       /* sanity check */
>                       if (length != 0 && length != elength)
>                         abort ();
> -                     return insn;
> +                     /* found, done */
> +                     break;
>                     }
>                 }
>             }
> @@ -530,10 +530,12 @@ cgen_lookup_insn (CGEN_CPU_DESC cd,
>          Could relax this later if it ever proves useful.  */
>        if (length == 0)
>         abort ();
> -      return insn;
>      }
>
> -  return NULL;
> +  if (cd->int_insn_p)
> +    free (buf);
> +
> +  return insn;
>  }
>
>  /* Fill in the operand instances used by INSN whose operands are FIELDS.
> --
> 2.9.3
>

-- 
Yao (齐尧)