Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)

To: Christos Zoulas <christos%astron.com@localhost>, tech-toolchain%netbsd.org@localhost
Subject: Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)
From: Kamil Rytarowski <n54%gmx.com@localhost>
Date: Sun, 23 Jun 2019 15:35:59 +0200

On 23.06.2019 05:44, Christos Zoulas wrote:
> In article <272cb7c0-c171-8b87-393a-0ccdd042b84b%gmx.com@localhost>,
> Kamil Rytarowski  <n54%gmx.com@localhost> wrote:
>> -=-=-=-=-=-
>> -=-=-=-=-=-
>>
>> On 22.06.2019 16:38, maya%netbsd.org@localhost wrote:
>>> The following kernel works with unmodified -current userland.
>>> NetBSD 7.99.75
>>>
>>> Reverting to the git mirror Dec 1 2018 with this list of commits also
>>> works.
>>>
>>> 4cab1e757c7197a01b6dc268c0104fa43ab7f0aa
>>> 5542798943417f4d1bdb61d0a49a9f8c1c37445c
>>> ef20f297e5f1527c1b54d4b2de8280a6cca21c71
>>> 80d084c7ddc1104b2bf191ccaa8a5347e71253ef
>>>
>>> We don't need any changes to GDB whatsoever. I didn't even rebuild the
>>> binary.
>>>
>>> It's a really tricky bisect because every few days changes are made to
>>> re-break it.
>>>
>>
>> My patch adds i386 to recognized osabi/bfd, but there is trash in
>> registers. Something gets malformed, might it be related to these
>> regressions?
> 
> Maxv disabled 64 bit ptrace on a 32 bit tracee. The following patch
> puts it back, and an unmodified gdb from head now can trace a 32 bit
> binary again.
> 
> Index: machdep.c
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/amd64/amd64/machdep.c,v
> retrieving revision 1.332
> diff -u -p -u -r1.332 machdep.c
> --- machdep.c	12 Jun 2019 14:28:38 -0000	1.332
> +++ machdep.c	23 Jun 2019 03:40:54 -0000
> @@ -2095,40 +2095,46 @@ cpu_setmcontext(struct lwp *l, const mco
>  int
>  cpu_mcontext_validate(struct lwp *l, const mcontext_t *mcp)
>  {
> -	struct proc *p __diagused = l->l_proc;
> +	struct proc *p = l->l_proc;
>  	struct trapframe *tf = l->l_md.md_regs;
>  	const __greg_t *gr;
>  	uint16_t sel;
> +	const bool pk32 = (p->p_flag & PK_32) != 0;
>  
> -	KASSERT((p->p_flag & PK_32) == 0);
>  	gr = mcp->__gregs;
>  
>  	if (((gr[_REG_RFLAGS] ^ tf->tf_rflags) & PSL_USERSTATIC) != 0)
>  		return EINVAL;
> +#define VUD(sel) (pk32 ? VALID_USER_DSEL32(sel) : VALID_USER_DSEL(sel))
> +#define VUF(sel) (pk32 ? (VALID_USER_DSEL32(sel) || VALID_USER_FSEL32(sel)) \
> +    : VALID_USER_DSEL(sel))
> +#define VUG(sel) (pk32 ? (VALID_USER_DSEL32(sel) || VALID_USER_GSEL32(sel)) \
> +    : VALID_USER_DSEL(sel))
> +#define VUC(sel) (pk32 ? VALID_USER_CSEL32(sel) : VALID_USER_CSEL(sel))
>  
>  	sel = gr[_REG_ES] & 0xffff;
> -	if (sel != 0 && !VALID_USER_DSEL(sel))
> +	if (sel != 0 && !VUD(sel))
>  		return EINVAL;
>  
>  	sel = gr[_REG_FS] & 0xffff;
> -	if (sel != 0 && !VALID_USER_DSEL(sel))
> +	if (sel != 0 && !VUF(sel))
>  		return EINVAL;
>  
>  	sel = gr[_REG_GS] & 0xffff;
> -	if (sel != 0 && !VALID_USER_DSEL(sel))
> +	if (sel != 0 && !VUG(sel))
>  		return EINVAL;
>  
>  	sel = gr[_REG_DS] & 0xffff;
> -	if (!VALID_USER_DSEL(sel))
> +	if (!VUD(sel))
>  		return EINVAL;
>  
>  #ifndef XENPV
>  	sel = gr[_REG_SS] & 0xffff;
> -	if (!VALID_USER_DSEL(sel))
> +	if (!VUD(sel))
>  		return EINVAL;
>  
>  	sel = gr[_REG_CS] & 0xffff;
> -	if (!VALID_USER_CSEL(sel))
> +	if (!VUC(sel))
>  		return EINVAL;
>  #endif
>  
> Index: process_machdep.c
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/amd64/amd64/process_machdep.c,v
> retrieving revision 1.39
> diff -u -p -u -r1.39 process_machdep.c
> --- process_machdep.c	11 Feb 2019 14:59:32 -0000	1.39
> +++ process_machdep.c	23 Jun 2019 03:40:54 -0000
> @@ -100,42 +100,48 @@ process_frame(struct lwp *l)
>  }
>  
>  int
> -process_read_regs(struct lwp *l, struct reg *regs)
> +process_read_regs(struct lwp *l, struct reg *regp)
>  {
>  	struct trapframe *tf = process_frame(l);
> -	struct proc *p = l->l_proc;
> +	long *regs = regp->regs;
> +	const bool pk32 = (l->l_proc->p_flag & PK_32) != 0;
>  
> -	if (p->p_flag & PK_32) {
> -		return EINVAL;
> -	}
> -
> -	regs->regs[_REG_RDI] = tf->tf_rdi;
> -	regs->regs[_REG_RSI] = tf->tf_rsi;
> -	regs->regs[_REG_RDX] = tf->tf_rdx;
> -	regs->regs[_REG_R10] = tf->tf_r10;
> -	regs->regs[_REG_R8]  = tf->tf_r8;
> -	regs->regs[_REG_R9]  = tf->tf_r9;
> +	regs[_REG_RDI] = tf->tf_rdi;
> +	regs[_REG_RSI] = tf->tf_rsi;
> +	regs[_REG_RDX] = tf->tf_rdx;
> +	regs[_REG_R10] = tf->tf_r10;
> +	regs[_REG_R8]  = tf->tf_r8;
> +	regs[_REG_R9]  = tf->tf_r9;
>  	/* argX not touched */
> -	regs->regs[_REG_RCX] = tf->tf_rcx;
> -	regs->regs[_REG_R11] = tf->tf_r11;
> -	regs->regs[_REG_R12] = tf->tf_r12;
> -	regs->regs[_REG_R13] = tf->tf_r13;
> -	regs->regs[_REG_R14] = tf->tf_r14;
> -	regs->regs[_REG_R15] = tf->tf_r15;
> -	regs->regs[_REG_RBP] = tf->tf_rbp;
> -	regs->regs[_REG_RBX] = tf->tf_rbx;
> -	regs->regs[_REG_RAX] = tf->tf_rax;
> -	regs->regs[_REG_GS]  = 0;
> -	regs->regs[_REG_FS]  = 0;
> -	regs->regs[_REG_ES]  = GSEL(GUDATA_SEL, SEL_UPL);
> -	regs->regs[_REG_DS]  = GSEL(GUDATA_SEL, SEL_UPL);
> -	regs->regs[_REG_TRAPNO] = tf->tf_trapno;
> -	regs->regs[_REG_ERR] = tf->tf_err;
> -	regs->regs[_REG_RIP] = tf->tf_rip;
> -	regs->regs[_REG_CS]  = LSEL(LUCODE_SEL, SEL_UPL);
> -	regs->regs[_REG_RFLAGS] = tf->tf_rflags;
> -	regs->regs[_REG_RSP] = tf->tf_rsp;
> -	regs->regs[_REG_SS]  = LSEL(LUDATA_SEL, SEL_UPL);
> +	regs[_REG_RCX] = tf->tf_rcx;
> +	regs[_REG_R11] = tf->tf_r11;
> +	regs[_REG_R12] = tf->tf_r12;
> +	regs[_REG_R13] = tf->tf_r13;
> +	regs[_REG_R14] = tf->tf_r14;
> +	regs[_REG_R15] = tf->tf_r15;
> +	regs[_REG_RBP] = tf->tf_rbp;
> +	regs[_REG_RBX] = tf->tf_rbx;
> +	regs[_REG_RAX] = tf->tf_rax;
> +	if (pk32) {
> +		regs[_REG_GS] = tf->tf_gs & 0xffff;
> +		regs[_REG_FS] = tf->tf_fs & 0xffff;
> +		regs[_REG_ES] = tf->tf_es & 0xffff;
> +		regs[_REG_DS] = tf->tf_ds & 0xffff;
> +		regs[_REG_CS] = tf->tf_cs & 0xffff;
> +		regs[_REG_SS] = tf->tf_ss & 0xffff;
> +	} else {
> +		regs[_REG_GS] = 0;
> +		regs[_REG_FS] = 0;
> +		regs[_REG_ES] = GSEL(GUDATA_SEL, SEL_UPL);
> +		regs[_REG_DS] = GSEL(GUDATA_SEL, SEL_UPL);
> +		regs[_REG_CS] = LSEL(LUCODE_SEL, SEL_UPL);
> +		regs[_REG_SS] = LSEL(LUDATA_SEL, SEL_UPL);
> +	}
> +	regs[_REG_TRAPNO] = tf->tf_trapno;
> +	regs[_REG_ERR] = tf->tf_err;
> +	regs[_REG_RIP] = tf->tf_rip;
> +	regs[_REG_RFLAGS] = tf->tf_rflags;
> +	regs[_REG_RSP] = tf->tf_rsp;
>  
>  	return 0;
>  }
> @@ -143,11 +149,6 @@ process_read_regs(struct lwp *l, struct 
>  int
>  process_read_fpregs(struct lwp *l, struct fpreg *regs, size_t *sz)
>  {
> -	struct proc *p = l->l_proc;
> -
> -	if (p->p_flag & PK_32) {
> -		return EINVAL;
> -	}
>  
>  	process_read_fpregs_xmm(l, &regs->fxstate);
>  
> @@ -157,11 +158,6 @@ process_read_fpregs(struct lwp *l, struc
>  int
>  process_read_dbregs(struct lwp *l, struct dbreg *regs, size_t *sz)
>  {
> -	struct proc *p = l->l_proc;
> -
> -	if (p->p_flag & PK_32) {
> -		return EINVAL;
> -	}
>  
>  	x86_dbregs_read(l, regs);
>  
> @@ -172,13 +168,9 @@ int
>  process_write_regs(struct lwp *l, const struct reg *regp)
>  {
>  	struct trapframe *tf = process_frame(l);
> -	struct proc *p = l->l_proc;
>  	int error;
>  	const long *regs = regp->regs;
> -
> -	if (p->p_flag & PK_32) {
> -		return EINVAL;
> -	}
> +	const bool pk32 = (l->l_proc->p_flag & PK_32) != 0;
>  
>  	/*
>  	 * Check for security violations. Note that struct regs is compatible
> @@ -204,16 +196,25 @@ process_write_regs(struct lwp *l, const 
>  	tf->tf_rbp  = regs[_REG_RBP];
>  	tf->tf_rbx  = regs[_REG_RBX];
>  	tf->tf_rax  = regs[_REG_RAX];
> -	tf->tf_gs   = 0;
> -	tf->tf_fs   = 0;
> -	tf->tf_es   = GSEL(GUDATA_SEL, SEL_UPL);
> -	tf->tf_ds   = GSEL(GUDATA_SEL, SEL_UPL);
> +	if (pk32) {
> +		tf->tf_gs = regs[_REG_GS] & 0xffff;
> +		tf->tf_fs = regs[_REG_FS] & 0xffff;
> +		tf->tf_es = regs[_REG_ES] & 0xffff;
> +		tf->tf_ds = regs[_REG_DS] & 0xffff;
> +		tf->tf_cs = regs[_REG_CS] & 0xffff;
> +		tf->tf_ss = regs[_REG_SS] & 0xffff;
> +	} else {
> +		tf->tf_gs = 0;
> +		tf->tf_fs = 0;
> +		tf->tf_es = GSEL(GUDATA_SEL, SEL_UPL);
> +		tf->tf_ds = GSEL(GUDATA_SEL, SEL_UPL);
> +		tf->tf_cs = LSEL(LUCODE_SEL, SEL_UPL);
> +		tf->tf_ss = LSEL(LUDATA_SEL, SEL_UPL);
> +	}
>  	/* trapno, err not touched */
>  	tf->tf_rip  = regs[_REG_RIP];
> -	tf->tf_cs   = LSEL(LUCODE_SEL, SEL_UPL);
>  	tf->tf_rflags = regs[_REG_RFLAGS];
>  	tf->tf_rsp  = regs[_REG_RSP];
> -	tf->tf_ss   = LSEL(LUDATA_SEL, SEL_UPL);
>  
>  #ifdef XENPV
>  	/* see comment in cpu_setmcontext */
> @@ -227,11 +228,6 @@ process_write_regs(struct lwp *l, const 
>  int
>  process_write_fpregs(struct lwp *l, const struct fpreg *regs, size_t sz)
>  {
> -	struct proc *p = l->l_proc;
> -
> -	if (p->p_flag & PK_32) {
> -		return EINVAL;
> -	}
>  
>  	process_write_fpregs_xmm(l, &regs->fxstate);
>  	return 0;
> @@ -240,13 +236,8 @@ process_write_fpregs(struct lwp *l, cons
>  int
>  process_write_dbregs(struct lwp *l, const struct dbreg *regs, size_t sz)
>  {
> -	struct proc *p = l->l_proc;
>  	int error;
>  
> -	if (p->p_flag & PK_32) {
> -		return EINVAL;
> -	}
> -
>  	/*
>  	 * Check for security violations.
>  	 */
> @@ -276,11 +267,6 @@ int
>  process_set_pc(struct lwp *l, void *addr)
>  {
>  	struct trapframe *tf = process_frame(l);
> -	struct proc *p = l->l_proc;
> -
> -	if (p->p_flag & PK_32) {
> -		return EINVAL;
> -	}
>  
>  	if ((uint64_t)addr >= VM_MAXUSER_ADDRESS)
>  		return EINVAL;
> 

We still want to pass FS/GS to userland through ptrace(2).

For PK_32 we could keep check for VM_MAXUSER_ADDRESS32.

Attachment: signature.asc
Description: OpenPGP digital signature

Follow-Ups:
- Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)
  - From: Christos Zoulas

References:
- Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)
  - From: Kamil Rytarowski
- Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)
  - From: maya
- Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)
  - From: Kamil Rytarowski
- Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)
  - From: Christos Zoulas

Prev by Date: Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)
Next by Date: Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)
Previous by Thread: Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)
Next by Thread: Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)
Indexes:

Home | Main Index | Thread Index | Old Index