[lkml]   [2010]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCHv11 2.6.36-rc2-tip 4/15] 4: uprobes: x86 specific functions for user space breakpointing.
    On Fri, 3 Sep 2010 23:18:32 +0530
    Srikar Dronamraju <> wrote:

    [cutting down cc list]

    > >
    > > One general comment here: since with uprobes the instruction
    > > decoder becomes security critical did you do any fuzz tests
    > > on it (e.g. like using it on crashme or on code that has
    > > been corrupted with a few bitflips) ?
    > I havent tried any fuzz tests with the instruction decoder. But I am
    > not sure if Masami has tried that out some of these.
    > One question: Do you want to test uprobes with crashme or test
    > instruction decoder with crashme.

    Ideally both, but as a minimum the part that is exposed
    to user space, that is uprobes.

    BTW if you test it I would test it both with real crashme
    and varying legal code that just has a few bits flipped.

    > > > +#ifdef CONFIG_X86_32
    > > > +#define is_32bit_app(tsk) 1
    > > > +#else
    > > > +#define is_32bit_app(tsk) (test_tsk_thread_flag(tsk, TIF_IA32))
    > > > +#endif
    > >
    > > This probably should be elsewhere.
    > Would this fit in x86 Instruction decoder?

    compat.h probably.

    > Okay, I can move the printk to the caller, I will try to shorten the
    > message, Would something like "uprobes: no support for 2-byte
    > opcode 0x0f 0x%2" look fine?

    Yes that's fine. Optionally you could supply a short
    script like scripts/decodecode that feeds it through objdump -d
    This might need dumping a few more bytes.

    > > This check is not fully correct because it's valid to have
    > > 32bit code in 64bit programs and vice versa. The only good
    > > way to check that is to look at the code segment at runtime
    > > though (and it gets complicated if you want to handle LDTs,
    > > but that could be optional). May be difficult to do though.
    > validate_insn_32bit is able to identify all valid instructions in a 32
    > bit app and validate_insn_64bits is a superset of
    > validate_insn_32bits; i.e it considers valid 32 bit codes as valid
    > too.

    How can this be? e.g. 32bit has 1 byte INC/DEC but on 64bit
    these are REX prefixes and can be in front of nearly anything.
    So a super set cannot be correct. It has to be either / or.

    > Did you get a chance to look at
    > validate_insn_32bit/validate_insn_64bits? If you feel that
    > validate_insn_32bit/validate_insn_64bits? are unable to detect
    > valid codes, then I will certainly rework.

    I don't think you can do a 100% solution because for 100%
    you would need to know the code segment the CPU is going
    to use later, and that's not possible in advance.

    A heuristic is reasonable (and leave out applications
    that generate 64bit code from 32bit executables or vice versa)
    but you need to test the right personality bits for that.

    > > Also the compat bit is not necessarily set if no system call is
    > > executing. You would rather need to check the exec_domain.
    > Okay, I shall check and revert on this.

    Hmm actually I double checked and this is a separate bit.
    So scratch that, TIF_32BIT is ok to test.

    -- -- Speaking for myself only.

     \ /
      Last update: 2010-09-06 09:55    [W:0.069 / U:1.588 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site