uprobes: __create_xol_area() must nullify xol_mapping.fault

As Jiri pointed out, the recent commit f872f5400cc0 "mm: Add a vm_special_mapping.fault() method" breaks uprobes. __create_xol_area() doesn't initialize the new ->fault() method and this obviously leads to kernel crash when the application tries to execute the probed insn after bp hit.

We probably want to add uprobes_special_mapping_fault(), this allows to turn xol_area->xol_mapping into a single instance of vm_special_mapping. But we need a simple fix, so lets change __create_xol() to nullify the new member as Jiri suggests.

Reported-by Jiri Olsa <[email protected]>

Suggested-by Jiri Olsa <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c | 1 +
 1 file changed, 1 insertion(+)

kernel/events/uprobes.c +0 -0

index 0167679..5f6ce93 100644

kernel/events/uprobes.c +0 -0

 	area->pages[0] = alloc_page(GFP_HIGHUSER);
 	if (!area->pages[0])

kernel/events/uprobes.c static struct xol_area *__create_xol_area(unsigned long vaddr) · line 1182

Quoted 8 lines · Oleg Nesterov

kernel/events/uprobes.c +1 -0

@@ -1178,6 +1178,7 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 		goto free_area;
 
 	area->xol_mapping.name = "[uprobes]";
+	area->xol_mapping.fault = NULL;
 	area->xol_mapping.pages = area->pages;

Would not something like:

	area->xol_mapping = (struct vm_special_mapping){
		.name = "[uprobes]",
		.pages = area->pages,
	};

Be a more robust approach? That way, if someone adds more fields, they at least get initialized (to 0).

Quoted 13 lines · Peter Zijlstra

kernel/events/uprobes.c static struct xol_area *__create_xol_area(unsigned long vaddr) · line 1182

Quoted 8 lines · Oleg Nesterov

kernel/events/uprobes.c +1 -0

@@ -1178,6 +1178,7 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 		goto free_area;
 
 	area->xol_mapping.name = "[uprobes]";
+	area->xol_mapping.fault = NULL;
 	area->xol_mapping.pages = area->pages;

Would not something like:

Quoted 7 lines

	area->xol_mapping = (struct vm_special_mapping){
		.name = "[uprobes]",
		.pages = area->pages,
	};

Be a more robust approach? That way, if someone adds more fields, they at least get initialized (to 0).

OK, agreed...

Do you want me to send v2? Or incremental patch because this one is already in -tip tree.

Or do nothing unless you feel strongly about it. area->xol_mapping should go away, but we need a simple preparation in mm/mmap.c.