diff mbox series

arm64: ptrace: Fix missing return in hw breakpoint code

Message ID 20210202002109.GA624440@juliacomputing.com (mailing list archive)
State New, archived
Headers show
Series arm64: ptrace: Fix missing return in hw breakpoint code | expand

Commit Message

Keno Fischer Feb. 2, 2021, 12:21 a.m. UTC
This was found by inspection - I don't work with aarch32 very
much, but I believe there is a missing return in this code,
since the second force_sig_fault here would override the
first, thus making the whole compat code here pointless.

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---

This was left over in my working tree from when I was working
through various aarch64 ptrace weirdness last year and I don't
think I ever got around to submitting it, but I just looked
through my work from back then and came across this again -
my apologies if I'm wrong about this - it's been a while,
so I'm not really read into this code anymore.

 arch/arm64/kernel/ptrace.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Will Deacon Feb. 2, 2021, 8:31 p.m. UTC | #1
Hi Keno,

On Mon, Feb 01, 2021 at 07:21:09PM -0500, Keno Fischer wrote:
> This was found by inspection - I don't work with aarch32 very
> much, but I believe there is a missing return in this code,
> since the second force_sig_fault here would override the
> first, thus making the whole compat code here pointless.
> 
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---
> 
> This was left over in my working tree from when I was working
> through various aarch64 ptrace weirdness last year and I don't
> think I ever got around to submitting it, but I just looked
> through my work from back then and came across this again -
> my apologies if I'm wrong about this - it's been a while,
> so I'm not really read into this code anymore.
> 
>  arch/arm64/kernel/ptrace.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index e7b01904f180..9309c41dbafe 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -195,6 +195,7 @@ static void ptrace_hbptriggered(struct perf_event *bp,
>  		arm64_force_sig_ptrace_errno_trap(si_errno,
>  						  (void __user *)bkpt->trigger,
>  						  desc);
> +		return;
>  	}
>  #endif
>  	arm64_force_sig_fault(SIGTRAP, TRAP_HWBKPT,

Thanks, I'll apply this for 5.12. However, I was trying to rework the commit
message to describe the impact of the issue and I'm not sure I agree with
your assessment that the second signal overrides the first. My (admittedly
brief) reading of __send_signal() is that the subsequent signal is
effectively ignored. Am I missing something?

Cheers,

Will
Keno Fischer Feb. 2, 2021, 9:01 p.m. UTC | #2
Hi Will,

> Thanks, I'll apply this for 5.12. However, I was trying to rework the commit
> message to describe the impact of the issue and I'm not sure I agree with
> your assessment that the second signal overrides the first. My (admittedly
> brief) reading of __send_signal() is that the subsequent signal is
> effectively ignored. Am I missing something?

Ah, I suppose you're probably right. I hadn't looked at this code in quite
a while, so I must have mis-remembered what the impact was.

Keno
diff mbox series

Patch

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index e7b01904f180..9309c41dbafe 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -195,6 +195,7 @@  static void ptrace_hbptriggered(struct perf_event *bp,
 		arm64_force_sig_ptrace_errno_trap(si_errno,
 						  (void __user *)bkpt->trigger,
 						  desc);
+		return;
 	}
 #endif
 	arm64_force_sig_fault(SIGTRAP, TRAP_HWBKPT,