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

[Luca Barbieri <ldb@ldb.ods.org>]

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

>  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.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]