netdev - Re: [PATCH iproute2 v7 1/3] ss: add support for BPF socket-local storage

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240212085913.7b158d41@hermes.local>
Date: Mon, 12 Feb 2024 08:59:13 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Quentin Deslandes <qde@...cy.de>
Cc: netdev@...r.kernel.org, David Ahern <dsahern@...il.com>, Martin KaFai
 Lau <martin.lau@...nel.org>, kernel-team@...a.com, Matthieu Baerts
 <matttbe@...nel.org>
Subject: Re: [PATCH iproute2 v7 1/3] ss: add support for BPF socket-local
 storage

On Mon, 12 Feb 2024 16:43:29 +0100
Quentin Deslandes <qde@...cy.de> wrote:

> While sock_diag is able to return BPF socket-local storage in response
> to INET_DIAG_REQ_SK_BPF_STORAGES requests, ss doesn't request it.
> 
> This change introduces the --bpf-maps and --bpf-map-id= options to request
> BPF socket-local storage for all SK_STORAGE maps, or only specific ones.
> 
> The bigger part of this change will check the requested map IDs and
> ensure they are valid. The column COL_EXT is used to print the
> socket-local data into.
> 
> When --bpf-maps is used, ss will send an empty INET_DIAG_REQ_SK_BPF_STORAGES
> request, in return the kernel will send all the BPF socket-local storage
> entries for a given socket. The BTF data for each map is loaded on
> demand, as ss can't predict which map ID are used.
> 
> When --bpf-map-id=ID is used, a file descriptor to the requested maps is
> open to 1) ensure the map doesn't disappear before the data is printed,
> and 2) ensure the map type is BPF_MAP_TYPE_SK_STORAGE. The BTF data for
> each requested map is loaded before the request is sent to the kernel.
> 
> Signed-off-by: Quentin Deslandes <qde@...cy.de>
> Co-authored-by: Martin KaFai Lau <martin.lau@...nel.org>
> Acked-by: Martin KaFai Lau <martin.lau@...nel.org>
> ---
>  misc/ss.c | 262 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 259 insertions(+), 3 deletions(-)


Some checkpatch complaints here.

WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#123: 
When --bpf-maps is used, ss will send an empty INET_DIAG_REQ_SK_BPF_STORAGES

WARNING: Non-standard signature: Co-authored-by:
#134: 
Co-authored-by: Martin KaFai Lau <martin.lau@...nel.org>

WARNING: line length of 112 exceeds 100 columns
#189: FILE: misc/ss.c:3417:
+		fprintf(stderr, "ss: too many (> %u) BPF socket-local storage maps found, skipping map ID %u\n",

WARNING: Block comments use a trailing */ on a separate line
#286: FILE: misc/ss.c:3514:
+	 * a socket, no matter their map ID. */

WARNING: Block comments use a trailing */ on a separate line
#304: FILE: misc/ss.c:3532:
+	 * to avoid inserting specific map IDs into the request. */

total: 0 errors, 5 warnings, 344 lines checked