Hello,
Here is the v2 series of update of the kprobe blacklist
checking on arm64.
I found that some blacklist checking code were mis-placed in
arch_prepare_kprobe() and arch_within_kprobe_blacklist().
Since the blacklist just filters by symbol, smaller than the
symbol, like extable must be checked in arch_prepare_kprobe().
Also, all function (symbol) level check must be done by blacklist.
For arm64, it checks the extable entry address in blacklist
and exception/irqentry function in arch_prepare_kprobe().
And, RODATA check is unneeded since kernel/kprobes.c
already ensures the probe address is in kernel-text area.
In v2, I updated [1/4]'s description and added James'
Reviewed-by. Also, in this version, I added a patch which
uses arch_populate_kprobe_blacklist() instead of
arch_within_kprobe_blacklist() so that user can see the full
list of blacklisted symbols under the debugfs.
Changes in v2:
- [1/4] change description so that it make clear and add
James' Reviewed-by.
- [4/4] new patch.
Thank you,
---
Masami Hiramatsu (4):
arm64: kprobes: Move extable address check into arch_prepare_kprobe()
arm64: kprobes: Remove unneeded RODATA check
arm64: kprobes: Move exception_text check in blacklist
arm64: kprobes: Use arch_populate_kprobe_blacklist()
arch/arm64/kernel/probes/kprobes.c | 49 ++++++++++++++++++------------------
1 file changed, 24 insertions(+), 25 deletions(-)
--
Masami Hiramatsu (Linaro) <[email protected]>
Move extable address check into arch_prepare_kprobe() from
arch_within_kprobe_blacklist().
The blacklist is exposed via debugfs as a list of symbols.
The extable entries are smaller, so must be filtered out
by arch_prepare_kprobe().
Signed-off-by: Masami Hiramatsu <[email protected]>
Reviewed-by: James Morse <[email protected]>
---
Update in v2:
- Update commit message.
- Add Reviewed-by from James.
---
arch/arm64/kernel/probes/kprobes.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 2a5b338b2542..b2d4b7428ebc 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -102,6 +102,10 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
if (in_exception_text(probe_addr))
return -EINVAL;
+
+ if (search_exception_tables(probe_addr))
+ return -EINVAL;
+
if (probe_addr >= (unsigned long) __start_rodata &&
probe_addr <= (unsigned long) __end_rodata)
return -EINVAL;
@@ -477,8 +481,7 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
(addr >= (unsigned long)__entry_text_start &&
addr < (unsigned long)__entry_text_end) ||
(addr >= (unsigned long)__idmap_text_start &&
- addr < (unsigned long)__idmap_text_end) ||
- !!search_exception_tables(addr))
+ addr < (unsigned long)__idmap_text_end))
return true;
if (!is_kernel_in_hyp_mode()) {
Remove unneeded RODATA check from arch_prepare_kprobe().
Since check_kprobe_address_safe() already ensured that
the probe address is in kernel text, we don't need to
check whether the address in RODATA or not. That must
be always false.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
arch/arm64/kernel/probes/kprobes.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index b2d4b7428ebc..1dae500d0a81 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -91,8 +91,6 @@ static void __kprobes arch_simulate_insn(struct kprobe *p, struct pt_regs *regs)
int __kprobes arch_prepare_kprobe(struct kprobe *p)
{
unsigned long probe_addr = (unsigned long)p->addr;
- extern char __start_rodata[];
- extern char __end_rodata[];
if (probe_addr & 0x3)
return -EINVAL;
@@ -106,10 +104,6 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
if (search_exception_tables(probe_addr))
return -EINVAL;
- if (probe_addr >= (unsigned long) __start_rodata &&
- probe_addr <= (unsigned long) __end_rodata)
- return -EINVAL;
-
/* decode instruction */
switch (arm_kprobe_decode_insn(p->addr, &p->ainsn)) {
case INSN_REJECTED: /* insn not supported */
Move exception/irqentry text address check in blacklist,
since those are symbol based rejection.
If we prohibit probing on the symbols in exception_text,
those should be blacklisted.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
arch/arm64/kernel/probes/kprobes.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 1dae500d0a81..b9e9758b6534 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -98,9 +98,6 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
/* copy instruction */
p->opcode = le32_to_cpu(*p->addr);
- if (in_exception_text(probe_addr))
- return -EINVAL;
-
if (search_exception_tables(probe_addr))
return -EINVAL;
@@ -475,7 +472,8 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
(addr >= (unsigned long)__entry_text_start &&
addr < (unsigned long)__entry_text_end) ||
(addr >= (unsigned long)__idmap_text_start &&
- addr < (unsigned long)__idmap_text_end))
+ addr < (unsigned long)__idmap_text_end) ||
+ in_exception_text(addr))
return true;
if (!is_kernel_in_hyp_mode()) {
Use arch_populate_kprobe_blacklist() instead of
arch_within_kprobe_blacklist() so that we can see the full
blacklisted symbols under the debugfs.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
arch/arm64/kernel/probes/kprobes.c | 42 ++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index b9e9758b6534..6c066c34c8a4 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -465,26 +465,30 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
return DBG_HOOK_HANDLED;
}
-bool arch_within_kprobe_blacklist(unsigned long addr)
+int __init arch_populate_kprobe_blacklist(void)
{
- if ((addr >= (unsigned long)__kprobes_text_start &&
- addr < (unsigned long)__kprobes_text_end) ||
- (addr >= (unsigned long)__entry_text_start &&
- addr < (unsigned long)__entry_text_end) ||
- (addr >= (unsigned long)__idmap_text_start &&
- addr < (unsigned long)__idmap_text_end) ||
- in_exception_text(addr))
- return true;
-
- if (!is_kernel_in_hyp_mode()) {
- if ((addr >= (unsigned long)__hyp_text_start &&
- addr < (unsigned long)__hyp_text_end) ||
- (addr >= (unsigned long)__hyp_idmap_text_start &&
- addr < (unsigned long)__hyp_idmap_text_end))
- return true;
- }
-
- return false;
+ int ret;
+
+ ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start,
+ (unsigned long)__kprobes_text_end);
+ if (ret)
+ return ret;
+ ret = kprobe_add_area_blacklist((unsigned long)__entry_text_start,
+ (unsigned long)__entry_text_end);
+ if (ret)
+ return ret;
+ ret = kprobe_add_area_blacklist((unsigned long)__idmap_text_start,
+ (unsigned long)__idmap_text_end);
+ if (ret || is_kernel_in_hyp_mode())
+ return ret;
+
+ ret = kprobe_add_area_blacklist((unsigned long)__hyp_text_start,
+ (unsigned long)__hyp_text_end);
+ if (ret)
+ return ret;
+ ret = kprobe_add_area_blacklist((unsigned long)__hyp_idmap_text_start,
+ (unsigned long)__hyp_idmap_text_end);
+ return ret;
}
void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
On Tue, Jan 15, 2019 at 03:24:36PM +0900, Masami Hiramatsu wrote:
> Remove unneeded RODATA check from arch_prepare_kprobe().
>
> Since check_kprobe_address_safe() already ensured that
> the probe address is in kernel text, we don't need to
> check whether the address in RODATA or not. That must
> be always false.
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
Reviewed-by: Mark Rutland <[email protected]>
Mark.
> ---
> arch/arm64/kernel/probes/kprobes.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index b2d4b7428ebc..1dae500d0a81 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -91,8 +91,6 @@ static void __kprobes arch_simulate_insn(struct kprobe *p, struct pt_regs *regs)
> int __kprobes arch_prepare_kprobe(struct kprobe *p)
> {
> unsigned long probe_addr = (unsigned long)p->addr;
> - extern char __start_rodata[];
> - extern char __end_rodata[];
>
> if (probe_addr & 0x3)
> return -EINVAL;
> @@ -106,10 +104,6 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
> if (search_exception_tables(probe_addr))
> return -EINVAL;
>
> - if (probe_addr >= (unsigned long) __start_rodata &&
> - probe_addr <= (unsigned long) __end_rodata)
> - return -EINVAL;
> -
> /* decode instruction */
> switch (arm_kprobe_decode_insn(p->addr, &p->ainsn)) {
> case INSN_REJECTED: /* insn not supported */
>
Hi Masami,
On Tue, Jan 15, 2019 at 03:23:39PM +0900, Masami Hiramatsu wrote:
> Hello,
>
> Here is the v2 series of update of the kprobe blacklist
> checking on arm64.
>
> I found that some blacklist checking code were mis-placed in
> arch_prepare_kprobe() and arch_within_kprobe_blacklist().
> Since the blacklist just filters by symbol, smaller than the
> symbol, like extable must be checked in arch_prepare_kprobe().
> Also, all function (symbol) level check must be done by blacklist.
>
> For arm64, it checks the extable entry address in blacklist
> and exception/irqentry function in arch_prepare_kprobe().
> And, RODATA check is unneeded since kernel/kprobes.c
> already ensures the probe address is in kernel-text area.
>
> In v2, I updated [1/4]'s description and added James'
> Reviewed-by. Also, in this version, I added a patch which
> uses arch_populate_kprobe_blacklist() instead of
> arch_within_kprobe_blacklist() so that user can see the full
> list of blacklisted symbols under the debugfs.
Assuming these are targetting the arm64 tree, are you intending to get them
merged for 5.0?
Will
Hi Will,
On Wed, 16 Jan 2019 13:40:07 +0000
Will Deacon <[email protected]> wrote:
> Hi Masami,
>
> On Tue, Jan 15, 2019 at 03:23:39PM +0900, Masami Hiramatsu wrote:
> > Hello,
> >
> > Here is the v2 series of update of the kprobe blacklist
> > checking on arm64.
> >
> > I found that some blacklist checking code were mis-placed in
> > arch_prepare_kprobe() and arch_within_kprobe_blacklist().
> > Since the blacklist just filters by symbol, smaller than the
> > symbol, like extable must be checked in arch_prepare_kprobe().
> > Also, all function (symbol) level check must be done by blacklist.
> >
> > For arm64, it checks the extable entry address in blacklist
> > and exception/irqentry function in arch_prepare_kprobe().
> > And, RODATA check is unneeded since kernel/kprobes.c
> > already ensures the probe address is in kernel-text area.
> >
> > In v2, I updated [1/4]'s description and added James'
> > Reviewed-by. Also, in this version, I added a patch which
> > uses arch_populate_kprobe_blacklist() instead of
> > arch_within_kprobe_blacklist() so that user can see the full
> > list of blacklisted symbols under the debugfs.
>
> Assuming these are targetting the arm64 tree, are you intending to get them
> merged for 5.0?
No, I don't rush it, since these are not bugfix but just enhancements.
(User can see the blacklisted symbols precisely) If you think you are
easy to pick this to arm64/next, it is OK to me.
Thank you,
--
Masami Hiramatsu <[email protected]>
Hi,
On 15/01/2019 06:25, Masami Hiramatsu wrote:
> Move exception/irqentry text address check in blacklist,
> since those are symbol based rejection.
>
> If we prohibit probing on the symbols in exception_text,
> those should be blacklisted.
We need to blacklist this as its where do_debug_exception() lives, which kprobes
depends on for single-stepping.
Reviewed-by: James Morse <[email protected]>
Thanks!
James
Hello,
On 15/01/2019 06:25, Masami Hiramatsu wrote:
> Use arch_populate_kprobe_blacklist() instead of
> arch_within_kprobe_blacklist() so that we can see the full
> blacklisted symbols under the debugfs.
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index b9e9758b6534..6c066c34c8a4 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -465,26 +465,30 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
> return DBG_HOOK_HANDLED;
> }
>
> -bool arch_within_kprobe_blacklist(unsigned long addr)
> +int __init arch_populate_kprobe_blacklist(void)
> {
> - if ((addr >= (unsigned long)__kprobes_text_start &&
> - addr < (unsigned long)__kprobes_text_end) ||
> - (addr >= (unsigned long)__entry_text_start &&
> - addr < (unsigned long)__entry_text_end) ||
> - (addr >= (unsigned long)__idmap_text_start &&
> - addr < (unsigned long)__idmap_text_end) ||
> - in_exception_text(addr))
You added this one in the previous patch, but it disappears here.
> - return true;
> -
> - if (!is_kernel_in_hyp_mode()) {
> - if ((addr >= (unsigned long)__hyp_text_start &&
> - addr < (unsigned long)__hyp_text_end) ||
> - (addr >= (unsigned long)__hyp_idmap_text_start &&
> - addr < (unsigned long)__hyp_idmap_text_end))
> - return true;
> - }
> -
> - return false;
> + int ret;
> + ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start,
> + (unsigned long)__kprobes_text_end);
> + if (ret)
> + return ret;
Now that we have arch_populate_kprobe_blacklist(), does the arch-code need to
blacklist the kprobes section itself?
The weak arch_within_kprobe_blacklist() will test it at kprobe-load time, and
populate_kprobe_blacklist() adds it to the list before it calls
arch_populate_kprobe_blacklist().
Won't this result in duplicate entries?
> + ret = kprobe_add_area_blacklist((unsigned long)__entry_text_start,
> + (unsigned long)__entry_text_end);
> + if (ret)
> + return ret;
> + ret = kprobe_add_area_blacklist((unsigned long)__idmap_text_start,
> + (unsigned long)__idmap_text_end);
> + if (ret || is_kernel_in_hyp_mode())
> + return ret;
Hmmm, I think we have a bug here today.
This is saying we can kprobe KVM when we have VHE, because all of KVMs code runs
at the same exception-level as the kernel. Which is true...
But KVM switches VBAR_EL1, so if we run over one of kprobes BRK instructions,
we're going to hyp-panic, because KVM doesn't handle synchronous exceptions from
EL2.
The __hyp_text also contains the guest entry/exit code, which we mustn't probe,
even on VHE.
I think we should always blacklist the __hyp_text, and KVM should mark its
vhe-only functions with __kprobes. I'll post patches for this.
Thanks,
James
> + ret = kprobe_add_area_blacklist((unsigned long)__hyp_text_start,
> + (unsigned long)__hyp_text_end);
> + if (ret)
> + return ret;
> + ret = kprobe_add_area_blacklist((unsigned long)__hyp_idmap_text_start,
> + (unsigned long)__hyp_idmap_text_end);
> + return ret;
> }
>
> void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
>
On Mon, 21 Jan 2019 12:20:07 +0000
James Morse <[email protected]> wrote:
> Hello,
>
> On 15/01/2019 06:25, Masami Hiramatsu wrote:
> > Use arch_populate_kprobe_blacklist() instead of
> > arch_within_kprobe_blacklist() so that we can see the full
> > blacklisted symbols under the debugfs.
> > diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> > index b9e9758b6534..6c066c34c8a4 100644
> > --- a/arch/arm64/kernel/probes/kprobes.c
> > +++ b/arch/arm64/kernel/probes/kprobes.c
> > @@ -465,26 +465,30 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
> > return DBG_HOOK_HANDLED;
> > }
> >
> > -bool arch_within_kprobe_blacklist(unsigned long addr)
> > +int __init arch_populate_kprobe_blacklist(void)
> > {
> > - if ((addr >= (unsigned long)__kprobes_text_start &&
> > - addr < (unsigned long)__kprobes_text_end) ||
> > - (addr >= (unsigned long)__entry_text_start &&
> > - addr < (unsigned long)__entry_text_end) ||
> > - (addr >= (unsigned long)__idmap_text_start &&
> > - addr < (unsigned long)__idmap_text_end) ||
>
> > - in_exception_text(addr))
>
> You added this one in the previous patch, but it disappears here.
Yes, it is easy to explain how we transcribe from
arch_within_kprobe_blacklist() to arch_populate_kprobe_blacklist().
>
>
> > - return true;
> > -
> > - if (!is_kernel_in_hyp_mode()) {
> > - if ((addr >= (unsigned long)__hyp_text_start &&
> > - addr < (unsigned long)__hyp_text_end) ||
> > - (addr >= (unsigned long)__hyp_idmap_text_start &&
> > - addr < (unsigned long)__hyp_idmap_text_end))
> > - return true;
> > - }
> > -
> > - return false;
> > + int ret;
>
>
> > + ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start,
> > + (unsigned long)__kprobes_text_end);
> > + if (ret)
> > + return ret;
>
> Now that we have arch_populate_kprobe_blacklist(), does the arch-code need to
> blacklist the kprobes section itself?
Ah, good catch! No, we don't need it here. Sorry I worked on older patch.
I'll update it.
> The weak arch_within_kprobe_blacklist() will test it at kprobe-load time, and
> populate_kprobe_blacklist() adds it to the list before it calls
> arch_populate_kprobe_blacklist().
>
> Won't this result in duplicate entries?
yes, so it should not.
>
>
> > + ret = kprobe_add_area_blacklist((unsigned long)__entry_text_start,
> > + (unsigned long)__entry_text_end);
> > + if (ret)
> > + return ret;
>
> > + ret = kprobe_add_area_blacklist((unsigned long)__idmap_text_start,
> > + (unsigned long)__idmap_text_end);
>
> > + if (ret || is_kernel_in_hyp_mode())
> > + return ret;
>
>
> Hmmm, I think we have a bug here today.
OK.
>
> This is saying we can kprobe KVM when we have VHE, because all of KVMs code runs
> at the same exception-level as the kernel. Which is true...
> But KVM switches VBAR_EL1, so if we run over one of kprobes BRK instructions,
> we're going to hyp-panic, because KVM doesn't handle synchronous exceptions from
> EL2.
>
> The __hyp_text also contains the guest entry/exit code, which we mustn't probe,
> even on VHE.
Hmm, I'm not sure when the original code decided this. But it sounds reasonable.
>
> I think we should always blacklist the __hyp_text, and KVM should mark its
> vhe-only functions with __kprobes. I'll post patches for this.
OK, then I should wait for that, because this series is a kind of improvement.
But your's is a bugfix, that should be backported to stable.
Thank you,
>
>
> Thanks,
>
> James
>
>
> > + ret = kprobe_add_area_blacklist((unsigned long)__hyp_text_start,
> > + (unsigned long)__hyp_text_end);
> > + if (ret)
> > + return ret;
> > + ret = kprobe_add_area_blacklist((unsigned long)__hyp_idmap_text_start,
> > + (unsigned long)__hyp_idmap_text_end);
> > + return ret;
> > }
> >
> > void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
> >
>
--
Masami Hiramatsu <[email protected]>
Hi Masami,
On Mon, Jan 21, 2019 at 10:25:58PM +0900, Masami Hiramatsu wrote:
> On Mon, 21 Jan 2019 12:20:07 +0000
> James Morse <[email protected]> wrote:
> > On 15/01/2019 06:25, Masami Hiramatsu wrote:
> > > Use arch_populate_kprobe_blacklist() instead of
> > > arch_within_kprobe_blacklist() so that we can see the full
> > > blacklisted symbols under the debugfs.
> > > diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> > > index b9e9758b6534..6c066c34c8a4 100644
> > > --- a/arch/arm64/kernel/probes/kprobes.c
> > > +++ b/arch/arm64/kernel/probes/kprobes.c
> > > @@ -465,26 +465,30 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
> > > return DBG_HOOK_HANDLED;
> > > }
> > >
> > > -bool arch_within_kprobe_blacklist(unsigned long addr)
> > > +int __init arch_populate_kprobe_blacklist(void)
> > > {
> > > - if ((addr >= (unsigned long)__kprobes_text_start &&
> > > - addr < (unsigned long)__kprobes_text_end) ||
> > > - (addr >= (unsigned long)__entry_text_start &&
> > > - addr < (unsigned long)__entry_text_end) ||
> > > - (addr >= (unsigned long)__idmap_text_start &&
> > > - addr < (unsigned long)__idmap_text_end) ||
> >
> > > - in_exception_text(addr))
> >
> > You added this one in the previous patch, but it disappears here.
>
> Yes, it is easy to explain how we transcribe from
> arch_within_kprobe_blacklist() to arch_populate_kprobe_blacklist().
>
> >
> >
> > > - return true;
> > > -
> > > - if (!is_kernel_in_hyp_mode()) {
> > > - if ((addr >= (unsigned long)__hyp_text_start &&
> > > - addr < (unsigned long)__hyp_text_end) ||
> > > - (addr >= (unsigned long)__hyp_idmap_text_start &&
> > > - addr < (unsigned long)__hyp_idmap_text_end))
> > > - return true;
> > > - }
> > > -
> > > - return false;
> > > + int ret;
> >
> >
> > > + ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start,
> > > + (unsigned long)__kprobes_text_end);
> > > + if (ret)
> > > + return ret;
> >
> > Now that we have arch_populate_kprobe_blacklist(), does the arch-code need to
> > blacklist the kprobes section itself?
>
> Ah, good catch! No, we don't need it here. Sorry I worked on older patch.
> I'll update it.
Did you send a new version of this series? I can't seem to spot it in my
inbox.
Cheers,
Will
On Fri, 8 Feb 2019 09:15:19 +0000
Will Deacon <[email protected]> wrote:
> Hi Masami,
>
> On Mon, Jan 21, 2019 at 10:25:58PM +0900, Masami Hiramatsu wrote:
> > On Mon, 21 Jan 2019 12:20:07 +0000
> > James Morse <[email protected]> wrote:
> > > On 15/01/2019 06:25, Masami Hiramatsu wrote:
> > > > Use arch_populate_kprobe_blacklist() instead of
> > > > arch_within_kprobe_blacklist() so that we can see the full
> > > > blacklisted symbols under the debugfs.
> > > > diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> > > > index b9e9758b6534..6c066c34c8a4 100644
> > > > --- a/arch/arm64/kernel/probes/kprobes.c
> > > > +++ b/arch/arm64/kernel/probes/kprobes.c
> > > > @@ -465,26 +465,30 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
> > > > return DBG_HOOK_HANDLED;
> > > > }
> > > >
> > > > -bool arch_within_kprobe_blacklist(unsigned long addr)
> > > > +int __init arch_populate_kprobe_blacklist(void)
> > > > {
> > > > - if ((addr >= (unsigned long)__kprobes_text_start &&
> > > > - addr < (unsigned long)__kprobes_text_end) ||
> > > > - (addr >= (unsigned long)__entry_text_start &&
> > > > - addr < (unsigned long)__entry_text_end) ||
> > > > - (addr >= (unsigned long)__idmap_text_start &&
> > > > - addr < (unsigned long)__idmap_text_end) ||
> > >
> > > > - in_exception_text(addr))
> > >
> > > You added this one in the previous patch, but it disappears here.
> >
> > Yes, it is easy to explain how we transcribe from
> > arch_within_kprobe_blacklist() to arch_populate_kprobe_blacklist().
> >
> > >
> > >
> > > > - return true;
> > > > -
> > > > - if (!is_kernel_in_hyp_mode()) {
> > > > - if ((addr >= (unsigned long)__hyp_text_start &&
> > > > - addr < (unsigned long)__hyp_text_end) ||
> > > > - (addr >= (unsigned long)__hyp_idmap_text_start &&
> > > > - addr < (unsigned long)__hyp_idmap_text_end))
> > > > - return true;
> > > > - }
> > > > -
> > > > - return false;
> > > > + int ret;
> > >
> > >
> > > > + ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start,
> > > > + (unsigned long)__kprobes_text_end);
> > > > + if (ret)
> > > > + return ret;
> > >
> > > Now that we have arch_populate_kprobe_blacklist(), does the arch-code need to
> > > blacklist the kprobes section itself?
> >
> > Ah, good catch! No, we don't need it here. Sorry I worked on older patch.
> > I'll update it.
>
> Did you send a new version of this series? I can't seem to spot it in my
> inbox.
Ah, OK. I just waited for James' patch series,
https://patchwork.kernel.org/cover/10779489/
Are those merged? I'd like to move this series on that.
Thank you for ping! :)
--
Masami Hiramatsu <[email protected]>
[+Marc]
On Mon, Feb 11, 2019 at 10:10:23PM +0900, Masami Hiramatsu wrote:
> On Fri, 8 Feb 2019 09:15:19 +0000
> Will Deacon <[email protected]> wrote:
> > Did you send a new version of this series? I can't seem to spot it in my
> > inbox.
>
> Ah, OK. I just waited for James' patch series,
>
> https://patchwork.kernel.org/cover/10779489/
>
> Are those merged? I'd like to move this series on that.
Patches 2-4 are in mainline:
f7daa9c8fd19 arm64: hibernate: Clean the __hyp_text to PoC after resume
8fac5cbdfe0f arm64: hyp-stub: Forbid kprobing of the hyp-stub
f2b3d8566d81 arm64: kprobe: Always blacklist the KVM world-switch code
Patch 1 is queued via kvm-arm (also for 5.0) but it doesn't seem to have
landed yet.
Will
On 11/02/2019 15:58, Will Deacon wrote:
> [+Marc]
>
> On Mon, Feb 11, 2019 at 10:10:23PM +0900, Masami Hiramatsu wrote:
>> On Fri, 8 Feb 2019 09:15:19 +0000
>> Will Deacon <[email protected]> wrote:
>>> Did you send a new version of this series? I can't seem to spot it in my
>>> inbox.
>>
>> Ah, OK. I just waited for James' patch series,
>>
>> https://patchwork.kernel.org/cover/10779489/
>>
>> Are those merged? I'd like to move this series on that.
>
> Patches 2-4 are in mainline:
>
> f7daa9c8fd19 arm64: hibernate: Clean the __hyp_text to PoC after resume
> 8fac5cbdfe0f arm64: hyp-stub: Forbid kprobing of the hyp-stub
> f2b3d8566d81 arm64: kprobe: Always blacklist the KVM world-switch code
>
> Patch 1 is queued via kvm-arm (also for 5.0) but it doesn't seem to have
> landed yet.
It was part of the pull request sent on Thursday[1], but Paolo hasn't
pulled it yet.
Hopefully soon...
M.
[1] https://patchwork.kernel.org/patch/10801151/
--
Jazz is not dead. It just smells funny...
On Mon, 11 Feb 2019 16:05:17 +0000
Marc Zyngier <[email protected]> wrote:
> On 11/02/2019 15:58, Will Deacon wrote:
> > [+Marc]
> >
> > On Mon, Feb 11, 2019 at 10:10:23PM +0900, Masami Hiramatsu wrote:
> >> On Fri, 8 Feb 2019 09:15:19 +0000
> >> Will Deacon <[email protected]> wrote:
> >>> Did you send a new version of this series? I can't seem to spot it in my
> >>> inbox.
> >>
> >> Ah, OK. I just waited for James' patch series,
> >>
> >> https://patchwork.kernel.org/cover/10779489/
> >>
> >> Are those merged? I'd like to move this series on that.
> >
> > Patches 2-4 are in mainline:
> >
> > f7daa9c8fd19 arm64: hibernate: Clean the __hyp_text to PoC after resume
> > 8fac5cbdfe0f arm64: hyp-stub: Forbid kprobing of the hyp-stub
> > f2b3d8566d81 arm64: kprobe: Always blacklist the KVM world-switch code
> >
> > Patch 1 is queued via kvm-arm (also for 5.0) but it doesn't seem to have
> > landed yet.
>
> It was part of the pull request sent on Thursday[1], but Paolo hasn't
> pulled it yet.
>
> Hopefully soon...
OK, then I'll send updated series since Patch1 is independent from
this series.
Thank you,
--
Masami Hiramatsu <[email protected]>