lkml.org 
[lkml]   [2011]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 2.6.38-rc8-tip 6/20] 6: x86: analyze instruction and determine fixups.
    On Mon, 14 Mar 2011, Srikar Dronamraju wrote:
    > +/*
    > + * TODO:
    > + * - Where necessary, examine the modrm byte and allow only valid instructions
    > + * in the different Groups and fpu instructions.
    > + */
    > +
    > +static bool is_prefix_bad(struct insn *insn)
    > +{
    > + int i;
    > +
    > + for (i = 0; i < insn->prefixes.nbytes; i++) {
    > + switch (insn->prefixes.bytes[i]) {
    > + case 0x26: /*INAT_PFX_ES */
    > + case 0x2E: /*INAT_PFX_CS */
    > + case 0x36: /*INAT_PFX_DS */
    > + case 0x3E: /*INAT_PFX_SS */
    > + case 0xF0: /*INAT_PFX_LOCK */
    > + return 1;

    true

    > + }
    > + }
    > + return 0;

    false

    > +}

    > +static int validate_insn_32bits(struct uprobe *uprobe, struct insn *insn)
    > +{
    > + insn_init(insn, uprobe->insn, false);
    > +
    > + /* Skip good instruction prefixes; reject "bad" ones. */
    > + insn_get_opcode(insn);
    > + if (is_prefix_bad(insn)) {
    > + report_bad_prefix();
    > + return -EPERM;

    -ENOTSUPP perhaps. That's not a permission problem

    > + }

    > +/**
    > + * analyze_insn - instruction analysis including validity and fixups.
    > + * @tsk: the probed task.
    > + * @uprobe: the probepoint information.
    > + * Return 0 on success or a -ve number on error.
    > + */
    > +int analyze_insn(struct task_struct *tsk, struct uprobe *uprobe)
    > +{
    > + int ret;
    > + struct insn insn;
    > +
    > + uprobe->fixups = 0;
    > +#ifdef CONFIG_X86_64
    > + uprobe->arch_info.rip_rela_target_address = 0x0;
    > +#endif

    Please get rid of this #ifdef and use inlines (empty for 32bit)

    > +
    > + if (is_32bit_app(tsk))
    > + ret = validate_insn_32bits(uprobe, &insn);
    > + else
    > + ret = validate_insn_64bits(uprobe, &insn);
    > + if (ret != 0)
    > + return ret;
    > +#ifdef CONFIG_X86_64

    Ditto

    > + ret = handle_riprel_insn(uprobe, &insn);
    > + if (ret == -1)
    > + /* rip-relative; can't XOL */
    > + return 0;

    So we return -1 from handle_riprel_insn() and return success? Btw how
    deals handle_riprel_insn() with 32bit user space ?

    > +#endif
    > + prepare_fixups(uprobe, &insn);
    > + return 0;

    Thanks,

    tglx


    \
     
     \ /
      Last update: 2011-03-15 15:41    [W:0.031 / U:1.200 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site