Skip to content

Commit 9fc205b

Browse files
anakryikoborkmann
authored andcommitted
libbpf: Add sane strncpy alternative and use it internally
strncpy() has a notoriously error-prone semantics which makes GCC complain about it a lot (and quite often completely completely falsely at that). Instead of pleasing GCC all the time (-Wno-stringop-truncation is unfortunately only supported by GCC, so it's a bit too messy to just enable it in Makefile), add libbpf-internal libbpf_strlcpy() helper which follows what FreeBSD's strlcpy() does and what most people would expect from strncpy(): copies up to N-1 first bytes from source string into destination string and ensures zero-termination afterwards. Replace all the relevant uses of strncpy/strncat/memcpy in libbpf with libbpf_strlcpy(). This also fixes the issue reported by Emmanuel Deloget in xsk.c where memcpy() could access source string beyond its end. Fixes: 2f6324a (libbpf: Support shared umems between queues and devices) Reported-by: Emmanuel Deloget <emmanuel.deloget@eho.link> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Link: https://lore.kernel.org/bpf/20211211004043.2374068-1-andrii@kernel.org
1 parent 4581e67 commit 9fc205b

File tree

6 files changed

+31
-19
lines changed

6 files changed

+31
-19
lines changed

tools/lib/bpf/bpf.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ int bpf_map_create(enum bpf_map_type map_type,
112112

113113
attr.map_type = map_type;
114114
if (map_name)
115-
strncat(attr.map_name, map_name, sizeof(attr.map_name) - 1);
115+
libbpf_strlcpy(attr.map_name, map_name, sizeof(attr.map_name));
116116
attr.key_size = key_size;
117117
attr.value_size = value_size;
118118
attr.max_entries = max_entries;
@@ -271,7 +271,7 @@ int bpf_prog_load_v0_6_0(enum bpf_prog_type prog_type,
271271
attr.kern_version = OPTS_GET(opts, kern_version, 0);
272272

273273
if (prog_name)
274-
strncat(attr.prog_name, prog_name, sizeof(attr.prog_name) - 1);
274+
libbpf_strlcpy(attr.prog_name, prog_name, sizeof(attr.prog_name));
275275
attr.license = ptr_to_u64(license);
276276

277277
if (insn_cnt > UINT_MAX)

tools/lib/bpf/btf_dump.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2321,8 +2321,8 @@ int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
23212321
if (!opts->indent_str)
23222322
d->typed_dump->indent_str[0] = '\t';
23232323
else
2324-
strncat(d->typed_dump->indent_str, opts->indent_str,
2325-
sizeof(d->typed_dump->indent_str) - 1);
2324+
libbpf_strlcpy(d->typed_dump->indent_str, opts->indent_str,
2325+
sizeof(d->typed_dump->indent_str));
23262326

23272327
d->typed_dump->compact = OPTS_GET(opts, compact, false);
23282328
d->typed_dump->skip_names = OPTS_GET(opts, skip_names, false);

tools/lib/bpf/gen_loader.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -463,8 +463,7 @@ void bpf_gen__map_create(struct bpf_gen *gen,
463463
attr.map_flags = map_attr->map_flags;
464464
attr.map_extra = map_attr->map_extra;
465465
if (map_name)
466-
memcpy(attr.map_name, map_name,
467-
min((unsigned)strlen(map_name), BPF_OBJ_NAME_LEN - 1));
466+
libbpf_strlcpy(attr.map_name, map_name, sizeof(attr.map_name));
468467
attr.numa_node = map_attr->numa_node;
469468
attr.map_ifindex = map_attr->map_ifindex;
470469
attr.max_entries = max_entries;
@@ -970,8 +969,7 @@ void bpf_gen__prog_load(struct bpf_gen *gen,
970969
core_relos = add_data(gen, gen->core_relos,
971970
attr.core_relo_cnt * attr.core_relo_rec_size);
972971

973-
memcpy(attr.prog_name, prog_name,
974-
min((unsigned)strlen(prog_name), BPF_OBJ_NAME_LEN - 1));
972+
libbpf_strlcpy(attr.prog_name, prog_name, sizeof(attr.prog_name));
975973
prog_load_attr = add_data(gen, &attr, attr_size);
976974

977975
/* populate union bpf_attr with a pointer to license */

tools/lib/bpf/libbpf.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,12 +1201,10 @@ static struct bpf_object *bpf_object__new(const char *path,
12011201

12021202
strcpy(obj->path, path);
12031203
if (obj_name) {
1204-
strncpy(obj->name, obj_name, sizeof(obj->name) - 1);
1205-
obj->name[sizeof(obj->name) - 1] = 0;
1204+
libbpf_strlcpy(obj->name, obj_name, sizeof(obj->name));
12061205
} else {
12071206
/* Using basename() GNU version which doesn't modify arg. */
1208-
strncpy(obj->name, basename((void *)path),
1209-
sizeof(obj->name) - 1);
1207+
libbpf_strlcpy(obj->name, basename((void *)path), sizeof(obj->name));
12101208
end = strchr(obj->name, '.');
12111209
if (end)
12121210
*end = 0;
@@ -1358,7 +1356,7 @@ static int bpf_object__check_endianness(struct bpf_object *obj)
13581356
static int
13591357
bpf_object__init_license(struct bpf_object *obj, void *data, size_t size)
13601358
{
1361-
memcpy(obj->license, data, min(size, sizeof(obj->license) - 1));
1359+
libbpf_strlcpy(obj->license, data, sizeof(obj->license));
13621360
pr_debug("license of %s is %s\n", obj->path, obj->license);
13631361
return 0;
13641362
}

tools/lib/bpf/libbpf_internal.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,25 @@ static inline void *libbpf_reallocarray(void *ptr, size_t nmemb, size_t size)
169169
return realloc(ptr, total);
170170
}
171171

172+
/* Copy up to sz - 1 bytes from zero-terminated src string and ensure that dst
173+
* is zero-terminated string no matter what (unless sz == 0, in which case
174+
* it's a no-op). It's conceptually close to FreeBSD's strlcpy(), but differs
175+
* in what is returned. Given this is internal helper, it's trivial to extend
176+
* this, when necessary. Use this instead of strncpy inside libbpf source code.
177+
*/
178+
static inline void libbpf_strlcpy(char *dst, const char *src, size_t sz)
179+
{
180+
size_t i;
181+
182+
if (sz == 0)
183+
return;
184+
185+
sz--;
186+
for (i = 0; i < sz && src[i]; i++)
187+
dst[i] = src[i];
188+
dst[i] = '\0';
189+
}
190+
172191
struct btf;
173192
struct btf_type;
174193

tools/lib/bpf/xsk.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -548,8 +548,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
548548
return -errno;
549549

550550
ifr.ifr_data = (void *)&channels;
551-
memcpy(ifr.ifr_name, ctx->ifname, IFNAMSIZ - 1);
552-
ifr.ifr_name[IFNAMSIZ - 1] = '\0';
551+
libbpf_strlcpy(ifr.ifr_name, ctx->ifname, IFNAMSIZ);
553552
err = ioctl(fd, SIOCETHTOOL, &ifr);
554553
if (err && errno != EOPNOTSUPP) {
555554
ret = -errno;
@@ -768,8 +767,7 @@ static int xsk_create_xsk_struct(int ifindex, struct xsk_socket *xsk)
768767
}
769768

770769
ctx->ifindex = ifindex;
771-
memcpy(ctx->ifname, ifname, IFNAMSIZ -1);
772-
ctx->ifname[IFNAMSIZ - 1] = 0;
770+
libbpf_strlcpy(ctx->ifname, ifname, IFNAMSIZ);
773771

774772
xsk->ctx = ctx;
775773
xsk->ctx->has_bpf_link = xsk_probe_bpf_link();
@@ -951,8 +949,7 @@ static struct xsk_ctx *xsk_create_ctx(struct xsk_socket *xsk,
951949
ctx->refcount = 1;
952950
ctx->umem = umem;
953951
ctx->queue_id = queue_id;
954-
memcpy(ctx->ifname, ifname, IFNAMSIZ - 1);
955-
ctx->ifname[IFNAMSIZ - 1] = '\0';
952+
libbpf_strlcpy(ctx->ifname, ifname, IFNAMSIZ);
956953

957954
ctx->fill = fill;
958955
ctx->comp = comp;

0 commit comments

Comments
 (0)