| 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
| ||
|
Message-ID: <20190805152936.GE4544@mini-arch>
Date: Mon, 5 Aug 2019 08:29:36 -0700
From: Stanislav Fomichev <sdf@...ichev.me>
To: Peter Wu <peter@...ensteyn.nl>
Cc: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
Stanislav Fomichev <sdf@...gle.com>,
Jakub Kicinski <jakub.kicinski@...ronome.com>,
Quentin Monnet <quentin.monnet@...ronome.com>
Subject: Re: [PATCH] tools: bpftool: fix reading from /proc/config.gz
On 08/05, Peter Wu wrote:
> /proc/config has never existed as far as I can see, but /proc/config.gz
> is present on Arch Linux. Execute an external gunzip program to avoid
> linking to zlib and rework the option scanning code since a pipe is not
> seekable. This also fixes a file handle leak on some error paths.
Thanks for doing that! One question: why not link against -lz instead?
With fork/execing gunzip you're just hiding this dependency.
You can add something like this to the Makefile:
ifeq ($(feature-zlib),1)
CLFAGS += -DHAVE_ZLIB
endif
And then conditionally add support for config.gz. Thoughts?
>
> Fixes: 4567b983f78c ("tools: bpftool: add probes for kernel configuration options")
> Cc: Quentin Monnet <quentin.monnet@...ronome.com>
> Signed-off-by: Peter Wu <peter@...ensteyn.nl>
> ---
> tools/bpf/bpftool/feature.c | 92 +++++++++++++++++++++----------------
> 1 file changed, 52 insertions(+), 40 deletions(-)
>
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index d672d9086fff..e9e10f582047 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -284,34 +284,34 @@ static void probe_jit_limit(void)
> }
> }
>
> -static char *get_kernel_config_option(FILE *fd, const char *option)
> +static bool get_kernel_config_option(FILE *fd, char **buf_p, size_t *n_p,
> + char **value)
> {
> - size_t line_n = 0, optlen = strlen(option);
> - char *res, *strval, *line = NULL;
> - ssize_t n;
> + char *sep;
> + ssize_t linelen;
>
> - rewind(fd);
> - while ((n = getline(&line, &line_n, fd)) > 0) {
> - if (strncmp(line, option, optlen))
> + while ((linelen = getline(buf_p, n_p, fd)) > 0) {
> + char *line = *buf_p;
> + if (strncmp(line, "CONFIG_", 7))
> continue;
> - /* Check we have at least '=', value, and '\n' */
> - if (strlen(line) < optlen + 3)
> - continue;
> - if (*(line + optlen) != '=')
> +
> + sep = memchr(line, '=', linelen);
> + if (!sep)
> continue;
>
> /* Trim ending '\n' */
> - line[strlen(line) - 1] = '\0';
> + line[linelen - 1] = '\0';
> +
> + /* Split on '=' and ensure that a value is present. */
> + *sep = '\0';
> + if (!sep[1])
> + continue;
>
> - /* Copy and return config option value */
> - strval = line + optlen + 1;
> - res = strdup(strval);
> - free(line);
> - return res;
> + *value = sep + 1;
> + return true;
> }
> - free(line);
>
> - return NULL;
> + return false;
> }
>
> static void probe_kernel_image_config(void)
> @@ -386,31 +386,34 @@ static void probe_kernel_image_config(void)
> /* test_bpf module for BPF tests */
> "CONFIG_TEST_BPF",
> };
> + char *values[ARRAY_SIZE(options)] = { };
> char *value, *buf = NULL;
> struct utsname utsn;
> char path[PATH_MAX];
> size_t i, n;
> ssize_t ret;
> - FILE *fd;
> + FILE *fd = NULL;
> + bool is_pipe = false;
>
> if (uname(&utsn))
> - goto no_config;
> + goto end_parse;
>
> snprintf(path, sizeof(path), "/boot/config-%s", utsn.release);
>
> fd = fopen(path, "r");
> if (!fd && errno == ENOENT) {
> - /* Some distributions put the config file at /proc/config, give
> - * it a try.
> - * Sometimes it is also at /proc/config.gz but we do not try
> - * this one for now, it would require linking against libz.
> + /* Some distributions build with CONFIG_IKCONFIG=y and put the
> + * config file at /proc/config.gz. We try to invoke an external
> + * gzip program to avoid linking to libz.
> + * Hide stderr to avoid interference with the JSON output.
> */
> - fd = fopen("/proc/config", "r");
> + fd = popen("gunzip -c /proc/config.gz 2>/dev/null", "r");
> + is_pipe = true;
> }
> if (!fd) {
> p_info("skipping kernel config, can't open file: %s",
> strerror(errno));
> - goto no_config;
> + goto end_parse;
> }
> /* Sanity checks */
> ret = getline(&buf, &n, fd);
> @@ -418,27 +421,36 @@ static void probe_kernel_image_config(void)
> if (!buf || !ret) {
> p_info("skipping kernel config, can't read from file: %s",
> strerror(errno));
> - free(buf);
> - goto no_config;
> + goto end_parse;
> }
> if (strcmp(buf, "# Automatically generated file; DO NOT EDIT.\n")) {
> p_info("skipping kernel config, can't find correct file");
> - free(buf);
> - goto no_config;
> + goto end_parse;
> + }
> +
> + while (get_kernel_config_option(fd, &buf, &n, &value)) {
> + for (i = 0; i < ARRAY_SIZE(options); i++) {
> + if (values[i] || strcmp(buf, options[i]))
> + continue;
> +
> + values[i] = strdup(value);
> + }
> + }
> +
> +end_parse:
> + if (fd) {
> + if (is_pipe) {
> + if (pclose(fd))
> + p_info("failed to read /proc/config.gz");
> + } else
> + fclose(fd);
> }
> free(buf);
>
> for (i = 0; i < ARRAY_SIZE(options); i++) {
> - value = get_kernel_config_option(fd, options[i]);
> - print_kernel_option(options[i], value);
> - free(value);
> + print_kernel_option(options[i], values[i]);
> + free(values[i]);
> }
> - fclose(fd);
> - return;
> -
> -no_config:
> - for (i = 0; i < ARRAY_SIZE(options); i++)
> - print_kernel_option(options[i], NULL);
> }
>
> static bool probe_bpf_syscall(const char *define_prefix)
> --
> 2.22.0
>
Powered by blists - more mailing lists