Re: [PATCH v3 3/3] arm64: hw_breakpoint: Save privilege of access control via ptrace

[Will Deacon]

On Tue, Jul 09, 2024 at 08:07:50PM +0800, Tiezhu Yang wrote:
> On 07/09/2024 06:05 PM, Will Deacon wrote:
> > On Tue, Jul 09, 2024 at 05:55:06PM +0800, Tiezhu Yang wrote:
> > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> > > index 0d022599eb61..3b37c4a2e0d4 100644
> > > --- a/arch/arm64/kernel/ptrace.c
> > > +++ b/arch/arm64/kernel/ptrace.c
> > > @@ -309,6 +309,7 @@ static struct perf_event *ptrace_hbp_create(unsigned int note_type,
> > >  	attr.bp_addr	= 0;
> > >  	attr.bp_len	= HW_BREAKPOINT_LEN_4;
> > >  	attr.bp_type	= type;
> > > +	attr.bp_priv	= AARCH64_BREAKPOINT_EL0;
> > >  	attr.disabled	= 1;
> > > 
> > >  	bp = register_user_hw_breakpoint(&attr, ptrace_hbptriggered, NULL, tsk);
> > > @@ -352,6 +353,7 @@ static int ptrace_hbp_fill_attr_ctrl(unsigned int note_type,
> > >  	attr->bp_len	= len;
> > >  	attr->bp_type	= type;
> > >  	attr->bp_addr	+= offset;
> > > +	attr->bp_priv	= ctrl.privilege;
> > 
> > Wait, so ptrace can now set breakpoints with arbitrary privileges?
> 
> The ptrace user should make sure the privilege is correct.
> For example, the privilege is set as el0
> in aarch64_point_encode_ctrl_reg() of GDB:
> 
>   /* enabled at el0 */
>   ctrl |= (2 << 1) | 1;
> 
> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/nat/aarch64-hw-point.c#l135

We shouldn't rely on userspace behaving correctly when dealing with
privilege levels.

Will