Re: [PATCH v2 bpf-next 08/13] resolve_btfids: Don't list an error that follows work on an argument

[Anton Protopopov <a.s.protopopov@gmail.com>]

On 26/11/13 05:08AM, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> A program that returns an error skips f(). A skip of f() is not a failure
> of f(). ksys_dup3() takes the lock and ends with "return do_dup2(..)":
> 
> 	static int do_dup2(..) __releases(&files->file_lock)
> 	..
> 	Ebusy:
> 		spin_unlock(&files->file_lock);
> 		return -EBUSY;
> 
> 	do_dup2() fails         dup2() = -EBUSY, the lock is free
> 	do_dup2() is skipped    dup2() = -EBUSY, the lock stays held
> 
> A caller can rely on what f() does on each path to "return e". The rule:
> 
> 	An error is not in the list if, on each path from the entry to its
> 	return, f() hands a pointer that is based on one of its pointer
> 	arguments to a callee that returns void.
> 
> A callee that returns void cannot fail: what it does to the object is done
> when f() returns the error. Most unlock, put, free and init functions
> return void. BTF says so, no name is needed. A callee that returns a value
> may have failed. "Based on": computed without a load, as &arg->lock is.
> 
> 	forward, may   the registers that hold a based pointer. A call that
> 	               hands one to a void callee is an effect. Does a path
> 	               without an effect lead from the entry to here?
> 	backward, may  the analysis that is there. It stops at an effect
> 
> f() stays a target if its list keeps an error.
> 
> 	                tagged            errors
> 	x86-64 GCC      12817 -> 11985    20967 -> 19248
> 	x86-64 clang    13549 -> 12633    22964 -> 20889
> 	arm64 clang     12435 -> 11543    21304 -> 19186
> The run on vmlinux takes 0.2 s more.
> 
> The tool does not see
>  - an unlock that is inlined: it is a store, not a call
>  - a callee that takes the object and returns a value, a computed callee
>  - a void callee without BTF: f.isra.0, a name that two functions have
>  - a pointer that was spilled to the stack, or loaded from the object
>  - that BUG() and panic() do not return: the next block counts as reached
> It also leaves out
>  - an error between mutex_lock(&arg->lock) and its unlock: both are void
>  - an error after a void callee that only prints a message about arg
> 
> Outline KASAN and KCSAN add void callees that get a pointer, such as
> __asan_load8(p) and __tsan_read8(p). Each read of *arg is an effect then:
> in 32 files of fs, net and mm 76 % of the errors go, not 6.5 %.
> Inline KASAN and KCOV: 6.3 % and 6.6 %. Targets are lost, none is added.
> 
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>

The robot's finding is real, otherwise

Acked-by: Anton Protopopov <a.s.protopopov@gmail.com>