On 5/18/20 3:00 PM, Jamal Hadi Salim wrote:
ping?
Note: these are trivial bug fixes.
Looking at c0325b06382c ("bpf: replace snprintf with asprintf when dealing with long
buffers"),
I wonder whether it's best to just revert and redo cleanly from scratch.. How
much testing has
been performed on the original patch? We know it is causing regressions, and
looking Jamal's
2nd patch we do have patterns all over the place wrt error path that go like:
+ char *file = NULL;
+ char buff[4096];
FILE *fp;
+ int ret;
- snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd);
+ ret = asprintf(&file, "/proc/%d/fdinfo/%d", getpid(), fd);
+ if (ret < 0) {
+ fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+ free(file);
+ return ret;
+ }
The man page on asprintf(char **strp, ...) says: "When successful, these
functions return
the number of bytes printed, just like sprintf(3). If memory allocation wasn't
possible,
or some other error occurs, these functions will return -1, and the contents of
strp are
undefined." What is the rationale that are we passing it to free() /everywhere/
in error
path when the API spec does say it's undefined? It may happen to work but
file's value
could just as well be, say, 42 ...
Thanks,
Daniel
cheers,
jamal
On 2020-04-28 12:15 p.m., Jamal Hadi Salim wrote:
Stephen,
What happened to this?
cheers,
jamal
On 2020-04-23 1:58 p.m., Jamal Hadi Salim wrote:
From: Jamal Hadi Salim <j...@mojatatu.com>
Changes from V2:
1) Dont initialize tmp on stack (Stephen)
2) Dont look at the return code of snprintf (Dominique)
3) Set errno to EINVAL instead of returning -EINVAL for consistency
(Dominique)
Changes from V1:
1) use snprintf instead of sprintf and fix corresponding error message.
Caught-by: Dominique Martinet <asmad...@codewreck.org>
2) Fix memory leak and extraneous free() in error path
Jamal Hadi Salim (2):
bpf: Fix segfault when custom pinning is used
bpf: Fix mem leak and extraneous free() in error path
lib/bpf.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
