* [PATCH v2] tools: bpftool: fix reading from /proc/config.gz
@ 2019-08-06 1:07 Peter Wu
2019-08-06 1:59 ` Jakub Kicinski
0 siblings, 1 reply; 2+ messages in thread
From: Peter Wu @ 2019-08-06 1:07 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, Stanislav Fomichev, Jakub Kicinski, Quentin Monnet
/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.
Fixes: 4567b983f78c ("tools: bpftool: add probes for kernel configuration options")
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
v2: fix style (reorder vars as reverse xmas tree, rename function,
braces), fallback to /proc/config.gz if uname() fails.
Hi,
Although Stanislav and Jakub suggested to use zlib in v1, I have not
implemented that yet since the current patch is quite minimal.
Using zlib instead of executing an external gzip program would like add
another 100-150 lines. It likely requires a bigger rewrite to avoid
getline() assuming that no temporary file is used for the uncompressed
config. If zlib is desired, I would suggest doing it in another patch.
Thoughts?
Kind regards,
Peter
---
tools/bpf/bpftool/feature.c | 104 +++++++++++++++++++++---------------
1 file changed, 60 insertions(+), 44 deletions(-)
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index d672d9086fff..b9ade5a8bc3c 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -284,34 +284,35 @@ static void probe_jit_limit(void)
}
}
-static char *get_kernel_config_option(FILE *fd, const char *option)
+static bool read_next_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;
+ ssize_t linelen;
+ char *sep;
- rewind(fd);
- while ((n = getline(&line, &line_n, fd)) > 0) {
- if (strncmp(line, option, optlen))
- continue;
- /* Check we have at least '=', value, and '\n' */
- if (strlen(line) < optlen + 3)
+ while ((linelen = getline(buf_p, n_p, fd)) > 0) {
+ char *line = *buf_p;
+
+ if (strncmp(line, "CONFIG_", 7))
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 +387,36 @@ 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];
+ bool is_pipe = false;
+ FILE *fd = NULL;
size_t i, n;
ssize_t ret;
- FILE *fd;
- if (uname(&utsn))
- goto no_config;
+ if (!uname(&utsn)) {
+ snprintf(path, sizeof(path), "/boot/config-%s", utsn.release);
- snprintf(path, sizeof(path), "/boot/config-%s", utsn.release);
+ fd = fopen(path, "r");
+ if (!fd && errno != ENOENT)
+ p_info("Cannot open %s: %s", path, strerror(errno));
+ }
- 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.
+ if (!fd) {
+ /* 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 +424,37 @@ 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 (read_next_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
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v2] tools: bpftool: fix reading from /proc/config.gz
2019-08-06 1:07 [PATCH v2] tools: bpftool: fix reading from /proc/config.gz Peter Wu
@ 2019-08-06 1:59 ` Jakub Kicinski
0 siblings, 0 replies; 2+ messages in thread
From: Jakub Kicinski @ 2019-08-06 1:59 UTC (permalink / raw)
To: Peter Wu
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Stanislav Fomichev,
Quentin Monnet
On Tue, 6 Aug 2019 02:07:02 +0100, 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.
Please post the fix for the handle leak separately against the bpf tree.
> Fixes: 4567b983f78c ("tools: bpftool: add probes for kernel configuration options")
Other than the leak I'm not sure this qualifies as a bug.
Reading config.gz was consciously left to be implemented later.
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
> v2: fix style (reorder vars as reverse xmas tree, rename function,
> braces), fallback to /proc/config.gz if uname() fails.
>
> Hi,
>
> Although Stanislav and Jakub suggested to use zlib in v1, I have not
> implemented that yet since the current patch is quite minimal.
>
> Using zlib instead of executing an external gzip program would like add
> another 100-150 lines. It likely requires a bigger rewrite to avoid
> getline() assuming that no temporary file is used for the uncompressed
> config. If zlib is desired, I would suggest doing it in another patch.
>
> Thoughts?
I'd rather avoid the fork and pipe. We already implicitly link against
zlib for libelf etc.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-08-06 2:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-06 1:07 [PATCH v2] tools: bpftool: fix reading from /proc/config.gz Peter Wu
2019-08-06 1:59 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox