On Tue, Mar 07, 2017 at 04:32:45PM -0800, Ricardo Neri wrote:
> The 32-bit and 64-bit address encodings are identical. This means that we
> can use the same function in both cases. In order to reuse the function for
> 32-bit address encodings, we must sign-extend our 32-bit signed operands to
> 64-bit signed variables (only for 64-bit builds). To decide on whether sign
> extension is needed, we rely on the address size as given by the
> instruction structure.
> 
> Lastly, before computing the linear address, we must truncate our signed
> 64-bit signed effective address if the address size is 32-bit.
> 
> Cc: Dave Hansen <dave.han...@linux.intel.com>
> Cc: Adam Buchbinder <adam.buchbin...@gmail.com>
> Cc: Colin Ian King <colin.k...@canonical.com>
> Cc: Lorenzo Stoakes <lstoa...@gmail.com>
> Cc: Qiaowei Ren <qiaowei....@intel.com>
> Cc: Arnaldo Carvalho de Melo <a...@redhat.com>
> Cc: Masami Hiramatsu <mhira...@kernel.org>
> Cc: Adrian Hunter <adrian.hun...@intel.com>
> Cc: Kees Cook <keesc...@chromium.org>
> Cc: Thomas Garnier <thgar...@google.com>
> Cc: Peter Zijlstra <pet...@infradead.org>
> Cc: Borislav Petkov <b...@suse.de>
> Cc: Dmitry Vyukov <dvyu...@google.com>
> Cc: Ravi V. Shankar <ravi.v.shan...@intel.com>
> Cc: x...@kernel.org
> Signed-off-by: Ricardo Neri <ricardo.neri-calde...@linux.intel.com>
> ---
>  arch/x86/lib/insn-eval.c | 44 ++++++++++++++++++++++++++++++++------------
>  1 file changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
> index edb360f..a9a1704 100644
> --- a/arch/x86/lib/insn-eval.c
> +++ b/arch/x86/lib/insn-eval.c
> @@ -559,6 +559,15 @@ int insn_get_reg_offset_sib_index(struct insn *insn, 
> struct pt_regs *regs)
>       return get_reg_offset(insn, regs, REG_TYPE_INDEX);
>  }
>  
> +static inline long __to_signed_long(unsigned long val, int long_bytes)
> +{
> +#ifdef CONFIG_X86_64
> +     return long_bytes == 4 ? (long)((int)((val) & 0xffffffff)) : (long)val;

I don't think this always works as expected:

---
typedef unsigned int u32;
typedef unsigned long u64;

int main()
{
        u64 v = 0x1ffffffff;

        printf("v: %ld, 0x%lx, %ld\n", v, v, (long)((int)((v) & 0xffffffff)));

        return 0;
}
--
...

v: 8589934591, 0x1ffffffff, -1

Now, this should not happen on 32-bit because unsigned long is 32-bit
there but can that happen on 64-bit?

> +#else
> +     return (long)val;
> +#endif
> +}
> +
>  /*
>   * return the address being referenced be instruction
>   * for rm=3 returning the content of the rm reg
> @@ -567,19 +576,21 @@ int insn_get_reg_offset_sib_index(struct insn *insn, 
> struct pt_regs *regs)
>  void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs)
>  {
>       unsigned long linear_addr, seg_base_addr;
> -     long eff_addr, base, indx;
> -     int addr_offset, base_offset, indx_offset;
> +     long eff_addr, base, indx, tmp;
> +     int addr_offset, base_offset, indx_offset, addr_bytes;
>       insn_byte_t sib;
>  
>       insn_get_modrm(insn);
>       insn_get_sib(insn);
>       sib = insn->sib.value;
> +     addr_bytes = insn->addr_bytes;
>  
>       if (X86_MODRM_MOD(insn->modrm.value) == 3) {
>               addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM);
>               if (addr_offset < 0)
>                       goto out_err;
> -             eff_addr = regs_get_register(regs, addr_offset);
> +             tmp = regs_get_register(regs, addr_offset);
> +             eff_addr = __to_signed_long(tmp, addr_bytes);

This repeats throughout the function so it begs to be a separate:

        get_mem_addr()

or so.

>               seg_base_addr = insn_get_seg_base(regs, insn, addr_offset,
>                                                 false);
>       } else {
> @@ -591,20 +602,24 @@ void __user *insn_get_addr_ref(struct insn *insn, 
> struct pt_regs *regs)
>                        * in the address computation.
>                        */
>                       base_offset = get_reg_offset(insn, regs, REG_TYPE_BASE);
> -                     if (unlikely(base_offset == -EDOM))
> +                     if (unlikely(base_offset == -EDOM)) {
>                               base = 0;
> -                     else if (unlikely(base_offset < 0))
> +                     } else if (unlikely(base_offset < 0)) {
>                               goto out_err;
> -                     else
> -                             base = regs_get_register(regs, base_offset);
> +                     } else {
> +                             tmp = regs_get_register(regs, base_offset);
> +                             base = __to_signed_long(tmp, addr_bytes);
> +                     }
>  
>                       indx_offset = get_reg_offset(insn, regs, 
> REG_TYPE_INDEX);
> -                     if (unlikely(indx_offset == -EDOM))
> +                     if (unlikely(indx_offset == -EDOM)) {
>                               indx = 0;
> -                     else if (unlikely(indx_offset < 0))
> +                     } else if (unlikely(indx_offset < 0)) {
>                               goto out_err;
> -                     else
> -                             indx = regs_get_register(regs, indx_offset);
> +                     } else {
> +                             tmp = regs_get_register(regs, indx_offset);
> +                             indx = __to_signed_long(tmp, addr_bytes);
> +                     }
>  
>                       eff_addr = base + indx * (1 << X86_SIB_SCALE(sib));
>                       seg_base_addr = insn_get_seg_base(regs, insn,
> @@ -625,13 +640,18 @@ void __user *insn_get_addr_ref(struct insn *insn, 
> struct pt_regs *regs)
>                       } else if (addr_offset < 0) {
>                               goto out_err;
>                       } else {
> -                             eff_addr = regs_get_register(regs, addr_offset);
> +                             tmp = regs_get_register(regs, addr_offset);
> +                             eff_addr = __to_signed_long(tmp, addr_bytes);
>                       }
>                       seg_base_addr = insn_get_seg_base(regs, insn,
>                                                         addr_offset, false);
>               }
>               eff_addr += insn->displacement.value;
>       }
> +     /* truncate to 4 bytes for 32-bit effective addresses */
> +     if (addr_bytes == 4)
> +             eff_addr &= 0xffffffff;

Why again?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imend├Ârffer, Jane Smithard, Graham Norton, HRB 21284 
(AG N├╝rnberg)
-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-msdos" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to