[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160926130359.GA9370@localhost.localdomain>
Date: Mon, 26 Sep 2016 18:33:59 +0530
From: Pratyush Anand <panand@...hat.com>
To: Catalin Marinas <catalin.marinas@....com>
Cc: Mark Rutland <mark.rutland@....com>, srikar@...ux.vnet.ibm.com,
Will Deacon <will.deacon@....com>,
Oleg Nesterov <oleg@...hat.com>,
Jungseok Lee <jungseoklee85@...il.com>,
Russell King - ARM Linux <linux@....linux.org.uk>,
vijaya.kumar@...iumnetworks.com, Dave Long <dave.long@...aro.org>,
Shi Yang <yang.shi@...aro.org>,
Vladimir Murzin <vladimir.murzin@....com>,
Steve Capper <steve.capper@...aro.org>,
"Suzuki K. Poulose" <suzuki.poulose@....com>,
Andre Przywara <andre.przywara@....com>,
Shaokun Zhang <zhangshaokun@...ilicon.com>,
Ashok Kumar <ashoks@...adcom.com>,
Sandeepa Prabhu <sandeepa.s.prabhu@...il.com>,
Will Cohen <wcohen@...hat.com>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
open list <linux-kernel@...r.kernel.org>,
James Morse <james.morse@....com>,
Masami Hiramatsu <mhiramat@...nel.org>,
Robin Murphy <robin.murphy@....com>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCH 5/5] arm64: Add uprobe support
On 26/09/2016:12:01:59 PM, Catalin Marinas wrote:
> On Sun, Sep 25, 2016 at 10:32:28PM +0530, Pratyush Anand wrote:
> > On Fri, Sep 23, 2016 at 6:35 PM, Catalin Marinas
> > <catalin.marinas@....com> wrote:
> > > On Fri, Sep 23, 2016 at 09:42:30AM +0530, Pratyush Anand wrote:
> > >> On 22/09/2016:05:50:30 PM, Catalin Marinas wrote:
> > >> > On Thu, Sep 22, 2016 at 08:53:28AM +0530, Pratyush Anand wrote:
> > >> > > On 21/09/2016:06:04:04 PM, Catalin Marinas wrote:
> >
> > >> > As a quick workaround you could check mm->task_size > TASK_SIZE_32 in
> > >> > the arch_uprobe_analyze_insn() function.
> > >>
> > >> It would be doable. TASK_SIZE_32 is defined only for COMPAT. So, may be I can
> > >> return -EINVAL when mm->task_size < TASK_SIZE_64.
> > >
> > > That's just a temporary workaround. If we ever merge ILP32, this test
> > > would no longer be enough (as the ISA is AArch64 but with TASK_SIZE_32).
> >
> > OK.. So what about doing something similar what x86 is doing.
> > We can have a flag for task Type in arch specific mm_context_t. We
> > also set this flag in COMPAT_SET_PERSONALITY() along with setting
> > thread_info flag, and we clear them in SET_PERSONALITY().
>
> This looks like a better approach.
>
> > > Looking at prepare_uprobe(), we have a weak is_trap_insn() function.
> > > This check is meaningless without knowing which instruction set we
> > > target. A false positive here, however, is not that bad as we wouldn't
> > > end up inserting the wrong breakpoint in the executable. But it looks to
> > > me like the core uprobe code needs to pass some additional information
> > > like the type of task or ELF format to the arch code to make a useful
> > > choice of breakpoint type.
> >
> > It seems that 'strtle r0, [r0], #160' would have the closest matching
> > aarch32 instruction wrt BRK64_OPCODE_UPROBES(0xd42000A0). But that too
> > seems a bad instruction. So, may be we can use still weak
> > is_trap_insn().
>
> Even if the is_trap_insn() check passes, we would reject the probe in
> arch_uprobe_analyze_insn() immediately after based on the mm type check,
> so not too bad.
OK..I will have an always returning false from arm64 is_trap_insn() in v2.
>
> If we add support for probing 32-bit tasks, I would rather have
> is_trap_insn() take the mm_struct as argument so that a non-weak
> implementation can check for the correct encoding.
Yes, for 32 bit task we would need mm_struct as arg in is_trap_insn() as well as
in is_swbp_insn(). We would also need to have arm64 specific set_swbp().
Thanks for all your input. It was helpful. I will send V2 soon.
~Pratyush
Powered by blists - more mailing lists