netdev - Re: [PATCH bpf-next 1/8] tools: bpftool: add basic probe capability, probe syscall and kversion

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]

Date:   Sat, 15 Dec 2018 00:35:38 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Quentin Monnet <quentin.monnet@...ronome.com>,
        Alexei Starovoitov <ast@...nel.org>
Cc:     netdev@...r.kernel.org, oss-drivers@...ronome.com,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Stanislav Fomichev <sdf@...gle.com>
Subject: Re: [PATCH bpf-next 1/8] tools: bpftool: add basic probe capability,
 probe syscall and kversion

On 12/13/2018 01:19 PM, Quentin Monnet wrote:
> Add a new component and command for bpftool, in order to probe the
> system to dump a set of eBPF-related parameters so that users can know
> what features are available on the system.
> 
> Parameters are dumped in plain or JSON output (with -j/-p options).
> Additionally, a specific keyword can be used to provide a third possible
> output so that the parameters are dumped as #define-d macros, ready to
> be saved to a header file and included in an eBPF-based project.
> 
> The current patch introduces probing of two simple parameters:
> availability of the bpf() system call, and kernel version. Later commits
> will add other probes.
> 
> Sample output:
> 
>     # bpftool feature probe kernel
>     Scanning system call and kernel version...
>     Kernel release is 4.19.0
>     bpf() syscall is available
> 
>     # bpftool --json --pretty feature probe kernel
>     {
>         "syscall_config": {
>             "kernel_version_code": 267008,
>             "have_bpf_syscall": true
>         }
>     }
> 
>     # bpftool feature probe kernel macros prefix BPFTOOL_
>     /*** System call and kernel version ***/
>     #define BPFTOOL_LINUX_VERSION_CODE 267008
>     #define BPFTOOL_BPF_SYSCALL
> 
> The optional "kernel" keyword enforces probing of the current system,
> which is the only possible behaviour at this stage. It can be safely
> omitted.
> 
> The feature comes with the relevant man page, but bash completion will
> come in a dedicated commit.
> 
> Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@...ronome.com>

First of all, thanks a lot for working on this infra!
Few comments below.

[...]
> +/* Probing functions */
> +
> +static int probe_kernel_version(const char *define_prefix)
> +{
> +	int version, subversion, patchlevel, code = 0;
> +	struct utsname utsn;
> +
> +	if (!uname(&utsn))
> +		if (sscanf(utsn.release, "%d.%d.%d",
> +			   &version, &subversion, &patchlevel) == 3)
> +			code = (version << 16) + (subversion << 8) + patchlevel;
> +
> +	if (json_output)
> +		jsonw_uint_field(json_wtr, "kernel_version_code", code);
> +	else if (define_prefix)
> +		printf("#define %sLINUX_VERSION_CODE %d\n",
> +		       define_prefix, code);
> +	else if (code)
> +		printf("Kernel release is %d.%d.%d\n",
> +		       version, subversion, patchlevel);
> +	else
> +		printf("Unable to parse kernel release number\n");
> +
> +	return code;
> +}

What would be the use-case to try to fetch the kernel version? My main
worry is that this doesn't tell much to the user e.g. in kernels where
features are mainly backported like RHEL. (Is it for the kprobes version
requirement?)

> +static bool probe_bpf_syscall(const char *define_prefix)
> +{
> +	bool res;
> +
> +	bpf_load_program(BPF_PROG_TYPE_UNSPEC, NULL, 0, NULL, 0, NULL, 0);
> +	res = (errno != ENOSYS);
> +
> +	print_bool_feature("have_bpf_syscall",
> +			   "BPF_SYSCALL",
> +			   "bpf() syscall",
> +			   res, define_prefix);
> +
> +	return res;
> +}