• David Vernet's avatar
    bpf: Don't use rcu_users to refcount in task kfuncs · 156ed20d
    David Vernet authored
    A series of prior patches added some kfuncs that allow struct
    task_struct * objects to be used as kptrs. These kfuncs leveraged the
    'refcount_t rcu_users' field of the task for performing refcounting.
    This field was used instead of 'refcount_t usage', as we wanted to
    leverage the safety provided by RCU for ensuring a task's lifetime.
    
    A struct task_struct is refcounted by two different refcount_t fields:
    
    1. p->usage:     The "true" refcount field which task lifetime. The
    		 task is freed as soon as this refcount drops to 0.
    
    2. p->rcu_users: An "RCU users" refcount field which is statically
    		 initialized to 2, and is co-located in a union with
    		 a struct rcu_head field (p->rcu). p->rcu_users
    		 essentially encapsulates a single p->usage
    		 refcount, and when p->rcu_users goes to 0, an RCU
    		 callback is scheduled on the struct rcu_head which
    		 decrements the p->usage refcount.
    
    Our logic was that by using p->rcu_users, we would be able to use RCU to
    safely issue refcount_inc_not_zero() a task's rcu_users field to
    determine if a task could still be acquired, or was exiting.
    Unfortunately, this does not work due to p->rcu_users and p->rcu sharing
    a union. When p->rcu_users goes to 0, an RCU callback is scheduled to
    drop a single p->usage refcount, and because the fields share a union,
    the refcount immediately becomes nonzero again after the callback is
    scheduled.
    
    If we were to split the fields out of the union, this wouldn't be a
    problem. Doing so should also be rather non-controversial, as there are
    a number of places in struct task_struct that have padding which we
    could use to avoid growing the structure by splitting up the fields.
    
    For now, so as to fix the kfuncs to be correct, this patch instead
    updates bpf_task_acquire() and bpf_task_release() to use the p->usage
    field for refcounting via the get_task_struct() and put_task_struct()
    functions. Because we can no longer rely on RCU, the change also guts
    the bpf_task_acquire_not_zero() and bpf_task_kptr_get() functions
    pending a resolution on the above problem.
    
    In addition, the task fixes the kfunc and rcu_read_lock selftests to
    expect this new behavior.
    
    Fixes: 90660309 ("bpf: Add kfuncs for storing struct task_struct * as a kptr")
    Fixes: fca1aa75 ("bpf: Handle MEM_RCU type properly")
    Reported-by: default avatarMatus Jokay <matus.jokay@stuba.sk>
    Signed-off-by: default avatarDavid Vernet <void@manifault.com>
    Link: https://lore.kernel.org/r/20221206210538.597606-1-void@manifault.comSigned-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
    156ed20d
task_kfunc_success.c 4.09 KB