* [PATCH iproute2-next] bpf: replace snprintf with asprintf when dealing with long buffers
@ 2019-09-09 16:05 Andrea Claudi
2019-09-15 17:45 ` David Ahern
0 siblings, 1 reply; 2+ messages in thread
From: Andrea Claudi @ 2019-09-09 16:05 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern
This reduces stack usage, as asprintf allocates memory on the heap.
This indirectly fixes a snprintf truncation warning (from gcc v9.2.1):
bpf.c: In function ‘bpf_get_work_dir’:
bpf.c:784:49: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
784 | snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
| ^
bpf.c:784:2: note: ‘snprintf’ output between 2 and 4097 bytes into a destination of size 4096
784 | snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fixes: e42256699cac ("bpf: make tc's bpf loader generic and move into lib")
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
lib/bpf.c | 95 +++++++++++++++++++++++++++++++++----------------------
1 file changed, 57 insertions(+), 38 deletions(-)
diff --git a/lib/bpf.c b/lib/bpf.c
index 7d2a322ffbaec..18e0334d3f11b 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -406,13 +406,14 @@ static int bpf_derive_elf_map_from_fdinfo(int fd, struct bpf_elf_map *map,
struct bpf_map_ext *ext)
{
unsigned int val, owner_type = 0, owner_jited = 0;
- char file[PATH_MAX], buff[4096];
+ char *file, buff[4096];
FILE *fp;
- snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd);
+ asprintf(&file, "/proc/%d/fdinfo/%d", getpid(), fd);
memset(map, 0, sizeof(*map));
fp = fopen(file, "r");
+ free(file);
if (!fp) {
fprintf(stderr, "No procfs support?!\n");
return -EIO;
@@ -600,7 +601,7 @@ int bpf_trace_pipe(void)
0,
};
int fd_in, fd_out = STDERR_FILENO;
- char tpipe[PATH_MAX];
+ char *tpipe;
const char *mnt;
mnt = bpf_find_mntpt("tracefs", TRACEFS_MAGIC, tracefs_mnt,
@@ -610,9 +611,10 @@ int bpf_trace_pipe(void)
return -1;
}
- snprintf(tpipe, sizeof(tpipe), "%s/trace_pipe", mnt);
+ asprintf(&tpipe, "%s/trace_pipe", mnt);
fd_in = open(tpipe, O_RDONLY);
+ free(tpipe);
if (fd_in < 0)
return -1;
@@ -633,37 +635,42 @@ int bpf_trace_pipe(void)
static int bpf_gen_global(const char *bpf_sub_dir)
{
- char bpf_glo_dir[PATH_MAX];
+ char *bpf_glo_dir;
int ret;
- snprintf(bpf_glo_dir, sizeof(bpf_glo_dir), "%s/%s/",
- bpf_sub_dir, BPF_DIR_GLOBALS);
+ asprintf(&bpf_glo_dir, "%s/%s/", bpf_sub_dir, BPF_DIR_GLOBALS);
ret = mkdir(bpf_glo_dir, S_IRWXU);
if (ret && errno != EEXIST) {
fprintf(stderr, "mkdir %s failed: %s\n", bpf_glo_dir,
strerror(errno));
- return ret;
+ goto out;
}
- return 0;
+ ret = 0;
+out:
+ free(bpf_glo_dir);
+ return ret;
}
static int bpf_gen_master(const char *base, const char *name)
{
- char bpf_sub_dir[PATH_MAX + NAME_MAX + 1];
+ char *bpf_sub_dir;
int ret;
- snprintf(bpf_sub_dir, sizeof(bpf_sub_dir), "%s%s/", base, name);
+ asprintf(&bpf_sub_dir, "%s%s/", base, name);
ret = mkdir(bpf_sub_dir, S_IRWXU);
if (ret && errno != EEXIST) {
fprintf(stderr, "mkdir %s failed: %s\n", bpf_sub_dir,
strerror(errno));
- return ret;
+ goto out;
}
- return bpf_gen_global(bpf_sub_dir);
+ ret = bpf_gen_global(bpf_sub_dir);
+out:
+ free(bpf_sub_dir);
+ return ret;
}
static int bpf_slave_via_bind_mnt(const char *full_name,
@@ -692,13 +699,13 @@ static int bpf_slave_via_bind_mnt(const char *full_name,
static int bpf_gen_slave(const char *base, const char *name,
const char *link)
{
- char bpf_lnk_dir[PATH_MAX + NAME_MAX + 1];
- char bpf_sub_dir[PATH_MAX + NAME_MAX];
+ char *bpf_lnk_dir;
+ char *bpf_sub_dir;
struct stat sb = {};
int ret;
- snprintf(bpf_lnk_dir, sizeof(bpf_lnk_dir), "%s%s/", base, link);
- snprintf(bpf_sub_dir, sizeof(bpf_sub_dir), "%s%s", base, name);
+ asprintf(&bpf_lnk_dir, "%s%s/", base, link);
+ asprintf(&bpf_sub_dir, "%s%s", base, name);
ret = symlink(bpf_lnk_dir, bpf_sub_dir);
if (ret) {
@@ -706,25 +713,30 @@ static int bpf_gen_slave(const char *base, const char *name,
if (errno != EPERM) {
fprintf(stderr, "symlink %s failed: %s\n",
bpf_sub_dir, strerror(errno));
- return ret;
+ goto out;
}
- return bpf_slave_via_bind_mnt(bpf_sub_dir,
- bpf_lnk_dir);
+ ret = bpf_slave_via_bind_mnt(bpf_sub_dir, bpf_lnk_dir);
+ goto out;
}
ret = lstat(bpf_sub_dir, &sb);
if (ret) {
fprintf(stderr, "lstat %s failed: %s\n",
bpf_sub_dir, strerror(errno));
- return ret;
+ goto out;
}
- if ((sb.st_mode & S_IFMT) != S_IFLNK)
- return bpf_gen_global(bpf_sub_dir);
+ if ((sb.st_mode & S_IFMT) != S_IFLNK) {
+ ret = bpf_gen_global(bpf_sub_dir);
+ goto out;
+ }
}
- return 0;
+out:
+ free(bpf_lnk_dir);
+ free(bpf_sub_dir);
+ return ret;
}
static int bpf_gen_hierarchy(const char *base)
@@ -742,7 +754,7 @@ static int bpf_gen_hierarchy(const char *base)
static const char *bpf_get_work_dir(enum bpf_prog_type type)
{
static char bpf_tmp[PATH_MAX] = BPF_DIR_MNT;
- static char bpf_wrk_dir[PATH_MAX];
+ static char *bpf_wrk_dir;
static const char *mnt;
static bool bpf_mnt_cached;
const char *mnt_env = getenv(BPF_ENV_MNT);
@@ -781,7 +793,7 @@ static const char *bpf_get_work_dir(enum bpf_prog_type type)
}
}
- snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
+ asprintf(&bpf_wrk_dir, "%s/", mnt);
ret = bpf_gen_hierarchy(bpf_wrk_dir);
if (ret) {
@@ -1438,29 +1450,31 @@ static int bpf_probe_pinned(const char *name, const struct bpf_elf_ctx *ctx,
static int bpf_make_obj_path(const struct bpf_elf_ctx *ctx)
{
- char tmp[PATH_MAX];
+ char *tmp;
int ret;
- snprintf(tmp, sizeof(tmp), "%s/%s", bpf_get_work_dir(ctx->type),
- ctx->obj_uid);
+ asprintf(&tmp, "%s/%s", bpf_get_work_dir(ctx->type), ctx->obj_uid);
ret = mkdir(tmp, S_IRWXU);
if (ret && errno != EEXIST) {
fprintf(stderr, "mkdir %s failed: %s\n", tmp, strerror(errno));
- return ret;
+ goto out;
}
- return 0;
+ ret = 0;
+out:
+ free(tmp);
+ return ret;
}
static int bpf_make_custom_path(const struct bpf_elf_ctx *ctx,
const char *todo)
{
- char tmp[PATH_MAX], rem[PATH_MAX], *sub;
+ char *tmp, *rem, *sub;
int ret;
- snprintf(tmp, sizeof(tmp), "%s/../", bpf_get_work_dir(ctx->type));
- snprintf(rem, sizeof(rem), "%s/", todo);
+ asprintf(&tmp, "%s/../", bpf_get_work_dir(ctx->type));
+ asprintf(&rem, "%s/", todo);
sub = strtok(rem, "/");
while (sub) {
@@ -1474,13 +1488,17 @@ static int bpf_make_custom_path(const struct bpf_elf_ctx *ctx,
if (ret && errno != EEXIST) {
fprintf(stderr, "mkdir %s failed: %s\n", tmp,
strerror(errno));
- return ret;
+ goto out;
}
sub = strtok(NULL, "/");
}
- return 0;
+ ret = 0;
+out:
+ free(rem);
+ free(tmp);
+ return ret;
}
static int bpf_place_pinned(int fd, const char *name,
@@ -2581,14 +2599,15 @@ struct bpf_jited_aux {
static int bpf_derive_prog_from_fdinfo(int fd, struct bpf_prog_data *prog)
{
- char file[PATH_MAX], buff[4096];
+ char *file, buff[4096];
unsigned int val;
FILE *fp;
- snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd);
+ asprintf(&file, "/proc/%d/fdinfo/%d", getpid(), fd);
memset(prog, 0, sizeof(*prog));
fp = fopen(file, "r");
+ free(file);
if (!fp) {
fprintf(stderr, "No procfs support?!\n");
return -EIO;
--
2.21.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH iproute2-next] bpf: replace snprintf with asprintf when dealing with long buffers
2019-09-09 16:05 [PATCH iproute2-next] bpf: replace snprintf with asprintf when dealing with long buffers Andrea Claudi
@ 2019-09-15 17:45 ` David Ahern
0 siblings, 0 replies; 2+ messages in thread
From: David Ahern @ 2019-09-15 17:45 UTC (permalink / raw)
To: Andrea Claudi, netdev; +Cc: stephen, dsahern
On 9/9/19 10:05 AM, Andrea Claudi wrote:
> This reduces stack usage, as asprintf allocates memory on the heap.
>
> This indirectly fixes a snprintf truncation warning (from gcc v9.2.1):
>
> bpf.c: In function ‘bpf_get_work_dir’:
> bpf.c:784:49: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
> 784 | snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
> | ^
> bpf.c:784:2: note: ‘snprintf’ output between 2 and 4097 bytes into a destination of size 4096
> 784 | snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fixes: e42256699cac ("bpf: make tc's bpf loader generic and move into lib")
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
> ---
> lib/bpf.c | 95 +++++++++++++++++++++++++++++++++----------------------
> 1 file changed, 57 insertions(+), 38 deletions(-)
>
> diff --git a/lib/bpf.c b/lib/bpf.c
> index 7d2a322ffbaec..18e0334d3f11b 100644
> --- a/lib/bpf.c
> +++ b/lib/bpf.c
> @@ -406,13 +406,14 @@ static int bpf_derive_elf_map_from_fdinfo(int fd, struct bpf_elf_map *map,
> struct bpf_map_ext *ext)
> {
> unsigned int val, owner_type = 0, owner_jited = 0;
> - char file[PATH_MAX], buff[4096];
> + char *file, buff[4096];
> FILE *fp;
>
> - snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd);
> + asprintf(&file, "/proc/%d/fdinfo/%d", getpid(), fd);
Thanks for taking this on.
From 'man asprintf':
"If memory allocation wasn't possible, or some other error occurs,
these functions will return -1, and the contents of strp are undefined."
ie., you should check the return code of asprintf and make sure the
pointer passed is always initialized to NULL (static ones do not need
the init).
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-09-15 17:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-09 16:05 [PATCH iproute2-next] bpf: replace snprintf with asprintf when dealing with long buffers Andrea Claudi
2019-09-15 17:45 ` David Ahern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.