netdev - Re: [PATCH bpf-next 5/6] libbpf: support libbpf-provided extern variables

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: <20191119032127.hixvyhvjjhx6mmzk@ast-mbp.dhcp.thefacebook.com>
Date:   Mon, 18 Nov 2019 19:21:29 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andriin@...com>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...com,
        daniel@...earbox.net, andrii.nakryiko@...il.com, kernel-team@...com
Subject: Re: [PATCH bpf-next 5/6] libbpf: support libbpf-provided extern
 variables

On Sat, Nov 16, 2019 at 11:08:06PM -0800, Andrii Nakryiko wrote:
> Add support for extern variables, provided to BPF program by libbpf. Currently
> the following extern variables are supported:
>   - LINUX_KERNEL_VERSION; version of a kernel in which BPF program is
>     executing, follows KERNEL_VERSION() macro convention;
>   - CONFIG_xxx values; a set of values of actual kernel config. Tristate,
>     boolean, and integer values are supported. Strings are not supported at
>     the moment.
> 
> All values are represented as 64-bit integers, with the follow value encoding:
>   - for boolean values, y is 1, n or missing value is 0;
>   - for tristate values, y is 1, m is 2, n or missing value is 0;
>   - for integers, the values is 64-bit integer, sign-extended, if negative; if
>     config value is missing, it's represented as 0, which makes explicit 0 and
>     missing config value indistinguishable. If this will turn out to be
>     a problem in practice, we'll need to deal with it somehow.

I read that statement as there is no extensibility for such api.

> Generally speaking, libbpf is not aware of which CONFIG_XXX values is of which
> expected type (bool, tristate, int), so it doesn't enforce any specific set of
> values and just parses n/y/m as 0/1/2, respectively. CONFIG_XXX values not
> found in config file are set to 0.

This is not pretty either.

> +
> +		switch (*value) {
> +		case 'n':
> +			*ext_val = 0;
> +			break;
> +		case 'y':
> +			*ext_val = 1;
> +			break;
> +		case 'm':
> +			*ext_val = 2;
> +			break;
> +		case '"':
> +			pr_warn("extern '%s': strings are not supported\n",
> +				ext->name);
> +			err = -EINVAL;
> +			goto out;
> +		default:
> +			errno = 0;
> +			*ext_val = strtoull(value, &value_end, 10);
> +			if (errno) {
> +				err = -errno;
> +				pr_warn("extern '%s': failed to parse value: %d\n",
> +					ext->name, err);
> +				goto out;
> +			}

BPF has bpf_strtol() helper. I think it would be cleaner to pass whatever
.config has as bytes to the program and let program parse n/y/m, strings and
integers.

LINUX_KERNEL_VERSION is a special case and can stay as u64.