netdev - Re: [PATCH v3 bpf-next 1/4] bpf: enable task local storage for tracing programs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACYkzJ4Ha1r4oqPge7yJjORdBUPg=huHSjER58ka24OEw_4S0A@mail.gmail.com>
Date:   Mon, 1 Feb 2021 02:20:46 +0100
From:   KP Singh <kpsingh@...nel.org>
To:     Song Liu <songliubraving@...com>
Cc:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>, mingo@...hat.com,
        Peter Zijlstra <peterz@...radead.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH v3 bpf-next 1/4] bpf: enable task local storage for
 tracing programs

On Thu, Jan 28, 2021 at 1:20 AM Song Liu <songliubraving@...com> wrote:
>
> To access per-task data, BPF programs usually creates a hash table with
> pid as the key. This is not ideal because:
>  1. The user need to estimate the proper size of the hash table, which may
>     be inaccurate;
>  2. Big hash tables are slow;
>  3. To clean up the data properly during task terminations, the user need
>     to write extra logic.
>
> Task local storage overcomes these issues and offers a better option for
> these per-task data. Task local storage is only available to BPF_LSM. Now
> enable it for tracing programs.
>
> Unlike LSM progreams, tracing programs can be called in IRQ contexts.

nit: typo *programs

> Helpers that accesses task local storage are updated to use

nit: Helpers that access..

> raw_spin_lock_irqsave() instead of raw_spin_lock_bh().
>
> Tracing programs can attach to functions on the task free path, e.g.
> exit_creds(). To avoid allocating task local storage after
> bpf_task_storage_free(). bpf_task_storage_get() is updated to not allocate
> new storage when the task is not refcounted (task->usage == 0).
>
> Signed-off-by: Song Liu <songliubraving@...com>

Acked-by: KP Singh <kpsingh@...nel.org>

Thanks for adding better commit descriptions :)

I think checking the usage before adding storage should work for the
task exit path (I could not think of cases where it would break).
Would also be nice to check with Martin and Hao about this.