Skip to content

Commit 8e7ae25

Browse files
iamkafaiborkmann
authored andcommitted
bpf: Sanitize the bpf_struct_ops tcp-cc name
The bpf_struct_ops tcp-cc name should be sanitized in order to avoid problematic chars (e.g. whitespaces). This patch reuses the bpf_obj_name_cpy() for accepting the same set of characters in order to keep a consistent bpf programming experience. A "size" param is added. Also, the strlen is returned on success so that the caller (like the bpf_tcp_ca here) can error out on empty name. The existing callers of the bpf_obj_name_cpy() only need to change the testing statement to "if (err < 0)". For all these existing callers, the err will be overwritten later, so no extra change is needed for the new strlen return value. v3: - reverse xmas tree style v2: - Save the orig_src to avoid "end - size" (Andrii) Fixes: 0baf26b ("bpf: tcp: Support tcp_congestion_ops in bpf") Signed-off-by: Martin KaFai Lau <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]> Acked-by: Andrii Nakryiko <[email protected]> Link: https://lore.kernel.org/bpf/[email protected]
1 parent 32ca98f commit 8e7ae25

File tree

3 files changed

+17
-16
lines changed

3 files changed

+17
-16
lines changed

include/linux/bpf.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ static inline void copy_map_value(struct bpf_map *map, void *dst, void *src)
160160
}
161161
void copy_map_value_locked(struct bpf_map *map, void *dst, void *src,
162162
bool lock_src);
163+
int bpf_obj_name_cpy(char *dst, const char *src, unsigned int size);
163164

164165
struct bpf_offload_dev;
165166
struct bpf_offloaded_map;

kernel/bpf/syscall.c

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -696,14 +696,15 @@ int bpf_get_file_flag(int flags)
696696
offsetof(union bpf_attr, CMD##_LAST_FIELD) - \
697697
sizeof(attr->CMD##_LAST_FIELD)) != NULL
698698

699-
/* dst and src must have at least BPF_OBJ_NAME_LEN number of bytes.
700-
* Return 0 on success and < 0 on error.
699+
/* dst and src must have at least "size" number of bytes.
700+
* Return strlen on success and < 0 on error.
701701
*/
702-
static int bpf_obj_name_cpy(char *dst, const char *src)
702+
int bpf_obj_name_cpy(char *dst, const char *src, unsigned int size)
703703
{
704-
const char *end = src + BPF_OBJ_NAME_LEN;
704+
const char *end = src + size;
705+
const char *orig_src = src;
705706

706-
memset(dst, 0, BPF_OBJ_NAME_LEN);
707+
memset(dst, 0, size);
707708
/* Copy all isalnum(), '_' and '.' chars. */
708709
while (src < end && *src) {
709710
if (!isalnum(*src) &&
@@ -712,11 +713,11 @@ static int bpf_obj_name_cpy(char *dst, const char *src)
712713
*dst++ = *src++;
713714
}
714715

715-
/* No '\0' found in BPF_OBJ_NAME_LEN number of bytes */
716+
/* No '\0' found in "size" number of bytes */
716717
if (src == end)
717718
return -EINVAL;
718719

719-
return 0;
720+
return src - orig_src;
720721
}
721722

722723
int map_check_no_btf(const struct bpf_map *map,
@@ -810,8 +811,9 @@ static int map_create(union bpf_attr *attr)
810811
if (IS_ERR(map))
811812
return PTR_ERR(map);
812813

813-
err = bpf_obj_name_cpy(map->name, attr->map_name);
814-
if (err)
814+
err = bpf_obj_name_cpy(map->name, attr->map_name,
815+
sizeof(attr->map_name));
816+
if (err < 0)
815817
goto free_map;
816818

817819
atomic64_set(&map->refcnt, 1);
@@ -2098,8 +2100,9 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
20982100
goto free_prog;
20992101

21002102
prog->aux->load_time = ktime_get_boottime_ns();
2101-
err = bpf_obj_name_cpy(prog->aux->name, attr->prog_name);
2102-
if (err)
2103+
err = bpf_obj_name_cpy(prog->aux->name, attr->prog_name,
2104+
sizeof(attr->prog_name));
2105+
if (err < 0)
21032106
goto free_prog;
21042107

21052108
/* run eBPF verifier */

net/ipv4/bpf_tcp_ca.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,6 @@ static int bpf_tcp_ca_init_member(const struct btf_type *t,
184184
{
185185
const struct tcp_congestion_ops *utcp_ca;
186186
struct tcp_congestion_ops *tcp_ca;
187-
size_t tcp_ca_name_len;
188187
int prog_fd;
189188
u32 moff;
190189

@@ -199,13 +198,11 @@ static int bpf_tcp_ca_init_member(const struct btf_type *t,
199198
tcp_ca->flags = utcp_ca->flags;
200199
return 1;
201200
case offsetof(struct tcp_congestion_ops, name):
202-
tcp_ca_name_len = strnlen(utcp_ca->name, sizeof(utcp_ca->name));
203-
if (!tcp_ca_name_len ||
204-
tcp_ca_name_len == sizeof(utcp_ca->name))
201+
if (bpf_obj_name_cpy(tcp_ca->name, utcp_ca->name,
202+
sizeof(tcp_ca->name)) <= 0)
205203
return -EINVAL;
206204
if (tcp_ca_find(utcp_ca->name))
207205
return -EEXIST;
208-
memcpy(tcp_ca->name, utcp_ca->name, sizeof(tcp_ca->name));
209206
return 1;
210207
}
211208

0 commit comments

Comments
 (0)