From: Catalin Marinas <catalin.marinas@arm.com> To: Pratyush Anand <panand@redhat.com> Cc: Mark Rutland <mark.rutland@arm.com>, srikar@linux.vnet.ibm.com, will.deacon@arm.com, oleg@redhat.com, Jungseok Lee <jungseoklee85@gmail.com>, linux@arm.linux.org.uk, vijaya.kumar@caviumnetworks.com, dave.long@linaro.org, Shi Yang <yang.shi@linaro.org>, Vladimir Murzin <vladimir.murzin@arm.com>, steve.capper@linaro.org, "Suzuki K. Poulose" <suzuki.poulose@arm.com>, Andre Przywara <andre.przywara@arm.com>, Shaokun Zhang <zhangshaokun@hisilicon.com>, Ashok Kumar <ashoks@broadcom.com>, Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com>, wcohen@redhat.com, linux-arm-kernel@lists.infradead.org, Ard Biesheuvel <ard.biesheuvel@linaro.org>, linux-kernel@vger.kernel.org, James Morse <james.morse@arm.com>, Masami Hiramatsu <mhiramat@kernel.org>, Robin Murphy <robin.murphy@arm.com>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> Subject: Re: [PATCH 5/5] arm64: Add uprobe support Date: Thu, 22 Sep 2016 17:50:30 +0100 [thread overview] Message-ID: <20160922165030.GA27704@e104818-lin.cambridge.arm.com> (raw) In-Reply-To: <20160922032328.GB29470@localhost.localdomain> On Thu, Sep 22, 2016 at 08:53:28AM +0530, Pratyush Anand wrote: > On 21/09/2016:06:04:04 PM, Catalin Marinas wrote: > > On Wed, Sep 21, 2016 at 04:30:47PM +0530, Pratyush Anand wrote: > > > On 20/09/2016:05:59:46 PM, Catalin Marinas wrote: > > > > > +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, > > > > > + unsigned long addr) > > > > > +{ > > > > > + probe_opcode_t insn; > > > > > + > > > > > + /* TODO: Currently we do not support AARCH32 instruction probing */ > > > > > > > > Is there a way to check (not necessarily in this file) that we don't > > > > probe 32-bit tasks? > > > > > > - Well, I do not have complete idea about it that, how it can be done. I think > > > we can not check that just by looking a single bit in an instruction. > > > My understanding is that, we can only know about it when we are executing the > > > instruction, by reading pstate, but that would not be useful for uprobe > > > instruction analysis. > > > > > > I hope, instruction encoding for aarch32 and aarch64 are different, and by > > > analyzing for all types of aarch32 instructions, we will be able to decide > > > that whether instruction is 32 bit trace-able or not. Accordingly, we can use > > > either BRK or BKPT instruction for breakpoint generation. > > > > We may have some unrelated instruction encoding overlapping but I > > haven't checked. I was more thinking about whether we know which task is > > being probed and check is_compat_task() or maybe using > > compat_user_mode(regs). > > I had thought of this, but problem is that we might not have task in existence > when we enable uprobes. For example: Lets say we are inserting a trace probe at > offset 0x690 in a executable binary. > > echo "p test:0x690" > /sys/kernel/debug/tracing/uprobe_events > echo 1 > /sys/kernel/debug/tracing/events/uprobes/enable > > In the 'enable' step, it is decided that whether instruction is traceable or > not. > > (1) But at this point 'test' executable might not be running. > (2) Even if it is running, is_compat_task() or compat_user_mode() might not be > usable, as they work with 'current' task. What I find strange is that uprobes allows you to insert a breakpoint instruction that's not even compatible with the task (so it would SIGILL rather than generate a debug exception). > What I was thinking that, let it go with 'TODO' as of now. Only that I don't have any guarantee that someone is going to fix it ;). As a quick workaround you could check mm->task_size > TASK_SIZE_32 in the arch_uprobe_analyze_insn() function. > Later on, we propose some changes in core layer, so that we can read the elf > headers of executable binary. ELFCLASS will be able to tell us, whether its a 32 > bit or 64 bit executable. I think, moving "struct uprobe" from > kernel/events/uprobes.c to a include/linux header file will do the job. "struct > arch_uprobe" is part of "struct uprobe". "struct arch_uprobe" is passed in > arch_uprobe_analyze_insn(). So, we can access struct uprobe's "inode" element > with this change. You can get access to struct linux_binfmt via mm_struct but it doesn't currently help much since all the members of this structure point to static functions. Maybe an enum in struct linux_binfmt with format types exposed to the rest of the kernel? -- Catalin
WARNING: multiple messages have this Message-ID (diff)
From: catalin.marinas@arm.com (Catalin Marinas) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 5/5] arm64: Add uprobe support Date: Thu, 22 Sep 2016 17:50:30 +0100 [thread overview] Message-ID: <20160922165030.GA27704@e104818-lin.cambridge.arm.com> (raw) In-Reply-To: <20160922032328.GB29470@localhost.localdomain> On Thu, Sep 22, 2016 at 08:53:28AM +0530, Pratyush Anand wrote: > On 21/09/2016:06:04:04 PM, Catalin Marinas wrote: > > On Wed, Sep 21, 2016 at 04:30:47PM +0530, Pratyush Anand wrote: > > > On 20/09/2016:05:59:46 PM, Catalin Marinas wrote: > > > > > +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, > > > > > + unsigned long addr) > > > > > +{ > > > > > + probe_opcode_t insn; > > > > > + > > > > > + /* TODO: Currently we do not support AARCH32 instruction probing */ > > > > > > > > Is there a way to check (not necessarily in this file) that we don't > > > > probe 32-bit tasks? > > > > > > - Well, I do not have complete idea about it that, how it can be done. I think > > > we can not check that just by looking a single bit in an instruction. > > > My understanding is that, we can only know about it when we are executing the > > > instruction, by reading pstate, but that would not be useful for uprobe > > > instruction analysis. > > > > > > I hope, instruction encoding for aarch32 and aarch64 are different, and by > > > analyzing for all types of aarch32 instructions, we will be able to decide > > > that whether instruction is 32 bit trace-able or not. Accordingly, we can use > > > either BRK or BKPT instruction for breakpoint generation. > > > > We may have some unrelated instruction encoding overlapping but I > > haven't checked. I was more thinking about whether we know which task is > > being probed and check is_compat_task() or maybe using > > compat_user_mode(regs). > > I had thought of this, but problem is that we might not have task in existence > when we enable uprobes. For example: Lets say we are inserting a trace probe at > offset 0x690 in a executable binary. > > echo "p test:0x690" > /sys/kernel/debug/tracing/uprobe_events > echo 1 > /sys/kernel/debug/tracing/events/uprobes/enable > > In the 'enable' step, it is decided that whether instruction is traceable or > not. > > (1) But at this point 'test' executable might not be running. > (2) Even if it is running, is_compat_task() or compat_user_mode() might not be > usable, as they work with 'current' task. What I find strange is that uprobes allows you to insert a breakpoint instruction that's not even compatible with the task (so it would SIGILL rather than generate a debug exception). > What I was thinking that, let it go with 'TODO' as of now. Only that I don't have any guarantee that someone is going to fix it ;). As a quick workaround you could check mm->task_size > TASK_SIZE_32 in the arch_uprobe_analyze_insn() function. > Later on, we propose some changes in core layer, so that we can read the elf > headers of executable binary. ELFCLASS will be able to tell us, whether its a 32 > bit or 64 bit executable. I think, moving "struct uprobe" from > kernel/events/uprobes.c to a include/linux header file will do the job. "struct > arch_uprobe" is part of "struct uprobe". "struct arch_uprobe" is passed in > arch_uprobe_analyze_insn(). So, we can access struct uprobe's "inode" element > with this change. You can get access to struct linux_binfmt via mm_struct but it doesn't currently help much since all the members of this structure point to static functions. Maybe an enum in struct linux_binfmt with format types exposed to the rest of the kernel? -- Catalin
next prev parent reply other threads:[~2016-09-22 16:50 UTC|newest] Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-08-02 5:30 [PATCH 0/5] ARM64: Uprobe support added Pratyush Anand 2016-08-02 5:30 ` Pratyush Anand 2016-08-02 5:30 ` [PATCH 1/5] arm64: kprobe: protect/rename few definitions to be reused by uprobe Pratyush Anand 2016-08-02 5:30 ` Pratyush Anand 2016-08-02 5:30 ` [PATCH 2/5] arm64: kgdb_step_brk_fn: ignore other's exception Pratyush Anand 2016-08-02 5:30 ` Pratyush Anand 2016-08-02 5:30 ` [PATCH 3/5] arm64: Handle TRAP_HWBRKPT for user mode as well Pratyush Anand 2016-08-02 5:30 ` Pratyush Anand 2016-09-06 16:11 ` Catalin Marinas 2016-09-06 16:11 ` Catalin Marinas 2016-09-06 21:36 ` David Long 2016-09-06 21:36 ` David Long 2016-09-07 4:47 ` Pratyush Anand 2016-09-07 4:47 ` Pratyush Anand 2016-09-07 13:41 ` Catalin Marinas 2016-09-07 13:41 ` Catalin Marinas 2016-08-02 5:30 ` [PATCH 4/5] arm64: Handle TRAP_BRKPT " Pratyush Anand 2016-08-02 5:30 ` Pratyush Anand 2016-09-06 16:34 ` Catalin Marinas 2016-09-06 16:34 ` Catalin Marinas 2016-08-02 5:30 ` [PATCH 5/5] arm64: Add uprobe support Pratyush Anand 2016-08-02 5:30 ` Pratyush Anand 2016-08-09 18:49 ` Oleg Nesterov 2016-08-09 18:49 ` Oleg Nesterov 2016-08-24 7:13 ` Pratyush Anand 2016-08-24 7:13 ` Pratyush Anand 2016-08-24 15:47 ` Oleg Nesterov 2016-08-24 15:47 ` Oleg Nesterov 2016-08-24 15:56 ` Will Deacon 2016-08-24 15:56 ` Will Deacon 2016-08-25 13:33 ` Oleg Nesterov 2016-08-25 13:33 ` Oleg Nesterov 2016-09-20 16:59 ` Catalin Marinas 2016-09-20 16:59 ` Catalin Marinas 2016-09-21 11:00 ` Pratyush Anand 2016-09-21 11:00 ` Pratyush Anand 2016-09-21 17:04 ` Catalin Marinas 2016-09-21 17:04 ` Catalin Marinas 2016-09-22 3:23 ` Pratyush Anand 2016-09-22 3:23 ` Pratyush Anand 2016-09-22 16:50 ` Catalin Marinas [this message] 2016-09-22 16:50 ` Catalin Marinas 2016-09-23 4:12 ` Pratyush Anand 2016-09-23 4:12 ` Pratyush Anand 2016-09-23 13:05 ` Catalin Marinas 2016-09-23 13:05 ` Catalin Marinas 2016-09-25 17:02 ` Pratyush Anand 2016-09-25 17:02 ` Pratyush Anand 2016-09-26 11:01 ` Catalin Marinas 2016-09-26 11:01 ` Catalin Marinas 2016-09-26 13:03 ` Pratyush Anand 2016-09-26 13:03 ` Pratyush Anand 2016-09-27 13:51 ` Catalin Marinas 2016-09-27 13:51 ` Catalin Marinas 2016-09-27 15:03 ` Pratyush Anand 2016-09-27 15:03 ` Pratyush Anand 2016-09-28 17:12 ` Catalin Marinas 2016-09-28 17:12 ` Catalin Marinas 2016-08-24 7:26 ` [PATCH 0/5] ARM64: Uprobe support added Pratyush Anand 2016-08-24 7:26 ` Pratyush Anand 2016-09-20 2:51 ` Pratyush Anand 2016-09-20 2:51 ` Pratyush Anand
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160922165030.GA27704@e104818-lin.cambridge.arm.com \ --to=catalin.marinas@arm.com \ --cc=andre.przywara@arm.com \ --cc=ard.biesheuvel@linaro.org \ --cc=ashoks@broadcom.com \ --cc=dave.long@linaro.org \ --cc=james.morse@arm.com \ --cc=jungseoklee85@gmail.com \ --cc=kirill.shutemov@linux.intel.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=mark.rutland@arm.com \ --cc=mhiramat@kernel.org \ --cc=oleg@redhat.com \ --cc=panand@redhat.com \ --cc=robin.murphy@arm.com \ --cc=sandeepa.s.prabhu@gmail.com \ --cc=srikar@linux.vnet.ibm.com \ --cc=steve.capper@linaro.org \ --cc=suzuki.poulose@arm.com \ --cc=vijaya.kumar@caviumnetworks.com \ --cc=vladimir.murzin@arm.com \ --cc=wcohen@redhat.com \ --cc=will.deacon@arm.com \ --cc=yang.shi@linaro.org \ --cc=zhangshaokun@hisilicon.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.