On Thu, Jun 1, 2023 at 5:26 PM KP Singh <kpsingh@xxxxxxxxxx> wrote:
>
> When task local storage was generalized for tracing programs, the
> bpf_task_local_storage callback was moved from a BPF LSM hook
> callback for security_task_free LSM hook to it's own callback. But a
> failure case in bad_fork_cleanup_security was missed which, when
> triggered, led to a dangling task owner pointer and a subsequent
> use-after-free. Move the bpf_task_storage_free to the very end of
> free_task to handle all failure cases.
>
> This issue was noticed when a BPF LSM program was attached to the
> task_alloc hook on a kernel with KASAN enabled. The program used
> bpf_task_storage_get to copy the task local storage from the current
> task to the new task being created.
>
> Fixes: a10787e6d58c ("bpf: Enable task local storage for tracing programs")
> Reported-by: Kuba Piecuch <jpiecuch@xxxxxxxxxx>
> Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx>
Acked-by: Song Liu <song@xxxxxxxxxx>
Thanks,
Song
> ---
>
> * v1 -> v2
>
> Move the bpf_task_storage_free to free_task as suggested by Martin
>
> kernel/fork.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index ed4e01daccaa..cb20f9f596d3 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -627,6 +627,7 @@ void free_task(struct task_struct *tsk)
> arch_release_task_struct(tsk);
> if (tsk->flags & PF_KTHREAD)
> free_kthread_struct(tsk);
> + bpf_task_storage_free(tsk);
> free_task_struct(tsk);
> }
> EXPORT_SYMBOL(free_task);
> @@ -979,7 +980,6 @@ void __put_task_struct(struct task_struct *tsk)
> cgroup_free(tsk);
> task_numa_free(tsk, true);
> security_task_free(tsk);
> - bpf_task_storage_free(tsk);
> exit_creds(tsk);
> delayacct_tsk_free(tsk);
> put_signal_struct(tsk->signal);
> --
> 2.41.0.rc0.172.g3f132b7071-goog
>
>
