Re: [PATCH] (re-xmit): kprobes for i386

[Luca Barbieri]

--=-vazCXNno2ygPvDXDJQXB
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

>  ENTRY(debug)
> +	pushl %eax
> +	SAVE_ALL
> +	movl %esp,%edx
>  	pushl $0
> -	pushl $do_debug
> -	jmp error_code
> +	pushl %edx
> +	call do_debug
> +	addl $8,%esp
> +	testl %eax,%eax 
> +	jnz restore_all
> +	jmp ret_from_exception
How about checking %cs in assembly and branching off for the kernel-mode
case?

Something like this:
ENTRY(debug)
	testl $0x3, 4(%esp)
	jz handle_kernel_mode_debug

> +	/*
> +	 * We singlestepped with interrupts disabled. So, the result on
> +	 * the stack would be incorrect for "pushfl" instruction.
> +	 */
> +	if (current_kprobe->opcode == 0x9c) { /* pushfl */
> +		regs->esp &= ~(TF_MASK | IF_MASK);
> +		regs->esp |= kprobe_old_eflags;
> +	}
This masks the stack pointer. It should mask the value pointer at by the
stack pointer.

> +	if (kprobe_running() && kprobe_fault_handler(regs, trapnr))
> +		return;
>  	if (!(regs->xcs & 3))
>  		goto kernel_trap;
The kprobe check should be after the kernel_trap label.

> +	if (kprobe_running() && kprobe_fault_handler(regs, 13))
> +		return;
>  
>  	if (!(regs->xcs & 3))
>  		goto gp_in_kernel;
Same here.

kernel. Therefore

> -	return;
> +	return 0;
Branching off in assembly would avoid having a return value in do_debug.

> +	if (kprobe_running() && kprobe_fault_handler(&regs, 7))
> +		return;
kprobe_running should be inline.

> +	if (kprobe_running() && kprobe_fault_handler(regs, 14))
> +		return;
> +
Same here.

--=-vazCXNno2ygPvDXDJQXB
Content-Type: application/pgp-signature; name=signature.asc
Content-Description: This is a digitally signed message part

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.7 (GNU/Linux)

iD8DBQA9Yi3wdjkty3ft5+cRAoOZAJ4iApQrRtUx6ZJJ+DZfOHYA/PjiCACfagrs
YG9Cto/UGJXVdK4t5VE+eCo=
=C7Pj
-----END PGP SIGNATURE-----

--=-vazCXNno2ygPvDXDJQXB--