[kprobe bug] pre-handler changes on globals lost

[Rusty Lynch <rusty dot lynch at intel dot com>]

While working with some kprobe unit test, I stumbled across some wacky
behavior that the below module exposes.

If you build and install this module, you can run tc1 (the working case)
by doing a "cat /proc/run_tc1".  You can then run the broken case by 
doing a "cat /proc/run_tc2".

When the module is installed a kprobe with a pre_handler that increments
a global counter is registered for a target function.

When the tescase is run it stores the current value of the global counter, 
calls a simple function (the target function that has been armed with a 
kprobe) that just returns the value of the global counter, and then the 
testcase verifies that the returned value has incremented (thanks to the 
kprobe.)

By adding a simple printk("peep\n"); to the target function, the effects
of the pre_handler on the global counter are lost.

Can someone else verify they see this behavior?  I see the bug on both i386
and x86_64 FC3 systems, but not on my ia64 REL4 system.
 
 -rusty

#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/kallsyms.h>
#include <linux/proc_fs.h>
#include <linux/kprobes.h>

static int pcount = 0;

/*
 * The only difference between target1 and target2 is that target1
 * calls printk before returning.
 */
int target1(void)
{
	/* adding a call makes the testcase work as expected */
	printk("peep\n");
	return pcount;
}
int target2(void)
{
	/* On i386 and x86_64 pcount does not get incremented in handler */
	return pcount;
}

/*
 * simple stuff... handler increments some global counter
 */
static int do_pre_handler(struct kprobe *p, struct pt_regs *regs)
{
	pcount++;
	return 0;
}

/*
 * kp1 and kp2 are identical
 */
static struct kprobe kp1 = {
	.pre_handler = do_pre_handler
};
static struct kprobe kp2 = {
	.pre_handler = do_pre_handler
};

/*
 * The only difference between show_run_tc1 and show_run_tc2 is that
 * one calls target1(), while the other calls target2()
 */
static int show_run_tc1(char *page, char **start, off_t offset,
			int count, int *eof, void *data)
{
	int initial = pcount;
	if (target1() != (initial + 1))
		return -EINVAL;

	return 0;
}
static int show_run_tc2(char *page, char **start, off_t offset,
			int count, int *eof, void *data)
{
	int initial = pcount;
	if (target2() != (initial + 1))
		return -EINVAL;

	return 0;
}

static int __init probe_test_init(void)
{
	struct proc_dir_entry *tc1_e, *tc2_e;

	tc1_e = create_proc_entry("run_tc1", S_IFREG|S_IRUGO, 0);
	if (!tc1_e)
		return -EIO;
	tc1_e->read_proc = show_run_tc1;

	tc2_e = create_proc_entry("run_tc2", S_IFREG|S_IRUGO, 0);
	if (!tc2_e)
		return -EIO;
	tc2_e->read_proc = show_run_tc2;


	kp1.addr = (kprobe_opcode_t *) kallsyms_lookup_name("target1");
	if (register_kprobe(&kp1))
		return -EINVAL;

	kp2.addr = (kprobe_opcode_t *) kallsyms_lookup_name("target2");
	if (register_kprobe(&kp2))
		return -EINVAL;

	return 0;
}

static void __exit probe_test_exit(void)
{
	unregister_kprobe(&kp2);
	unregister_kprobe(&kp1);
	remove_proc_entry("run_tc2", 0);
	remove_proc_entry("run_tc1", 0);
}

module_exit(probe_test_exit);
module_init(probe_test_init);
MODULE_LICENSE("GPL");