bpf: fix lockdep false positive in stackmap

Alexei Starovoitov · borkmann · commit 3defaf2f15b2 · 2019-02-11T16:36:24.000+01:00
Lockdep warns about false positive: [ 11.211460] ------------[ cut here ]------------ [ 11.211936] DEBUG_LOCKS_WARN_ON(depth <= 0) [ 11.211985] WARNING: CPU: 0 PID: 141 at ../kernel/locking/lockdep.c:3592 lock_release+0x1ad/0x280 [ 11.213134] Modules linked in: [ 11.214954] RIP: 0010:lock_release+0x1ad/0x280 [ 11.223508] Call Trace: [ 11.223705] <IRQ> [ 11.223874] ? __local_bh_enable+0x7a/0x80 [ 11.224199] up_read+0x1c/0xa0 [ 11.224446] do_up_read+0x12/0x20 [ 11.224713] irq_work_run_list+0x43/0x70 [ 11.225030] irq_work_run+0x26/0x50 [ 11.225310] smp_irq_work_interrupt+0x57/0x1f0 [ 11.225662] irq_work_interrupt+0xf/0x20 since rw_semaphore is released in a different task vs task that locked the sema. It is expected behavior. Fix the warning with up_read_non_owner() and rwsem_release() annotation. Fixes: bae77c5 ("bpf: enable stackmap with build_id in nmi context") Signed-off-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
@@ -44,7 +44,7 @@ static void do_up_read(struct irq_work *entry)
 	struct stack_map_irq_work *work;
 
 	work = container_of(entry, struct stack_map_irq_work, irq_work);
-	up_read(work->sem);
+	up_read_non_owner(work->sem);
 	work->sem = NULL;
 }
 
@@ -338,6 +338,12 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
 	} else {
 		work->sem = &current->mm->mmap_sem;
 		irq_work_queue(&work->irq_work);
+		/*
+		 * The irq_work will release the mmap_sem with
+		 * up_read_non_owner(). The rwsem_release() is called
+		 * here to release the lock from lockdep's perspective.
+		 */
+		rwsem_release(&current->mm->mmap_sem.dep_map, 1, _RET_IP_);
 	}
 }
 

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ static void do_up_read(struct irq_work *entry)`
`44`	`44`	`struct stack_map_irq_work *work;`
`45`	`45`
`46`	`46`	`work = container_of(entry, struct stack_map_irq_work, irq_work);`
`47`		`- up_read(work->sem);`
	`47`	`+ up_read_non_owner(work->sem);`
`48`	`48`	`work->sem = NULL;`
`49`	`49`	`}`
`50`	`50`
`@@ -338,6 +338,12 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,`
`338`	`338`	`} else {`
`339`	`339`	`work->sem = &current->mm->mmap_sem;`
`340`	`340`	`irq_work_queue(&work->irq_work);`
	`341`	`+ /*`
	`342`	`+ * The irq_work will release the mmap_sem with`
	`343`	`+ * up_read_non_owner(). The rwsem_release() is called`
	`344`	`+ * here to release the lock from lockdep's perspective.`
	`345`	`+ */`
	`346`	`+ rwsem_release(&current->mm->mmap_sem.dep_map, 1, _RET_IP_);`
`341`	`347`	`}`
`342`	`348`	`}`
`343`	`349`