• David Vernet's avatar
    bpf: Teach verifier that trusted PTR_TO_BTF_ID pointers are non-NULL · 51302c95
    David Vernet authored
    In reg_type_not_null(), we currently assume that a pointer may be NULL
    if it has the PTR_MAYBE_NULL modifier, or if it doesn't belong to one of
    several base type of pointers that are never NULL-able. For example,
    PTR_TO_CTX, PTR_TO_MAP_VALUE, etc.
    
    It turns out that in some cases, PTR_TO_BTF_ID can never be NULL as
    well, though we currently don't specify it. For example, if you had the
    following program:
    
    SEC("tc")
    long example_refcnt_fail(void *ctx)
    {
    	struct bpf_cpumask *mask1, *mask2;
    
    	mask1 = bpf_cpumask_create();
    	mask2 = bpf_cpumask_create();
    
            if (!mask1 || !mask2)
    		goto error_release;
    
    	bpf_cpumask_test_cpu(0, (const struct cpumask *)mask1);
    	bpf_cpumask_test_cpu(0, (const struct cpumask *)mask2);
    
    error_release:
    	if (mask1)
    		bpf_cpumask_release(mask1);
    	if (mask2)
    		bpf_cpumask_release(mask2);
    	return ret;
    }
    
    The verifier will incorrectly fail to load the program, thinking
    (unintuitively) that we have a possibly-unreleased reference if the mask
    is NULL, because we (correctly) don't issue a bpf_cpumask_release() on
    the NULL path.
    
    The reason the verifier gets confused is due to the fact that we don't
    explicitly tell the verifier that trusted PTR_TO_BTF_ID pointers can
    never be NULL. Basically, if we successfully get past the if check
    (meaning both pointers go from ptr_or_null_bpf_cpumask to
    ptr_bpf_cpumask), the verifier will correctly assume that the references
    need to be dropped on any possible branch that leads to program exit.
    However, it will _incorrectly_ think that the ptr == NULL branch is
    possible, and will erroneously detect it as a branch on which we failed
    to drop the reference.
    
    The solution is of course to teach the verifier that trusted
    PTR_TO_BTF_ID pointers can never be NULL, so that it doesn't incorrectly
    think it's possible for the reference to be present on the ptr == NULL
    branch.
    
    A follow-on patch will add a selftest that verifies this behavior.
    Signed-off-by: default avatarDavid Vernet <void@manifault.com>
    Link: https://lore.kernel.org/r/20230602150112.1494194-1-void@manifault.comSigned-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
    51302c95
verifier.c 567 KB