Commit 8e7ae251 authored by Martin KaFai Lau's avatar Martin KaFai Lau Committed by Daniel Borkmann

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: 0baf26b0 ("bpf: tcp: Support tcp_congestion_ops in bpf")
Signed-off-by: default avatarMartin KaFai Lau <kafai@fb.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarAndrii Nakryiko <andriin@fb.com>
Link: https://lore.kernel.org/bpf/20200314010209.1131542-1-kafai@fb.com
parent 32ca98fe
...@@ -160,6 +160,7 @@ static inline void copy_map_value(struct bpf_map *map, void *dst, void *src) ...@@ -160,6 +160,7 @@ static inline void copy_map_value(struct bpf_map *map, void *dst, void *src)
} }
void copy_map_value_locked(struct bpf_map *map, void *dst, void *src, void copy_map_value_locked(struct bpf_map *map, void *dst, void *src,
bool lock_src); bool lock_src);
int bpf_obj_name_cpy(char *dst, const char *src, unsigned int size);
struct bpf_offload_dev; struct bpf_offload_dev;
struct bpf_offloaded_map; struct bpf_offloaded_map;
......
...@@ -696,14 +696,15 @@ int bpf_get_file_flag(int flags) ...@@ -696,14 +696,15 @@ int bpf_get_file_flag(int flags)
offsetof(union bpf_attr, CMD##_LAST_FIELD) - \ offsetof(union bpf_attr, CMD##_LAST_FIELD) - \
sizeof(attr->CMD##_LAST_FIELD)) != NULL sizeof(attr->CMD##_LAST_FIELD)) != NULL
/* dst and src must have at least BPF_OBJ_NAME_LEN number of bytes. /* dst and src must have at least "size" number of bytes.
* Return 0 on success and < 0 on error. * Return strlen on success and < 0 on error.
*/ */
static int bpf_obj_name_cpy(char *dst, const char *src) int bpf_obj_name_cpy(char *dst, const char *src, unsigned int size)
{ {
const char *end = src + BPF_OBJ_NAME_LEN; const char *end = src + size;
const char *orig_src = src;
memset(dst, 0, BPF_OBJ_NAME_LEN); memset(dst, 0, size);
/* Copy all isalnum(), '_' and '.' chars. */ /* Copy all isalnum(), '_' and '.' chars. */
while (src < end && *src) { while (src < end && *src) {
if (!isalnum(*src) && if (!isalnum(*src) &&
...@@ -712,11 +713,11 @@ static int bpf_obj_name_cpy(char *dst, const char *src) ...@@ -712,11 +713,11 @@ static int bpf_obj_name_cpy(char *dst, const char *src)
*dst++ = *src++; *dst++ = *src++;
} }
/* No '\0' found in BPF_OBJ_NAME_LEN number of bytes */ /* No '\0' found in "size" number of bytes */
if (src == end) if (src == end)
return -EINVAL; return -EINVAL;
return 0; return src - orig_src;
} }
int map_check_no_btf(const struct bpf_map *map, int map_check_no_btf(const struct bpf_map *map,
...@@ -810,8 +811,9 @@ static int map_create(union bpf_attr *attr) ...@@ -810,8 +811,9 @@ static int map_create(union bpf_attr *attr)
if (IS_ERR(map)) if (IS_ERR(map))
return PTR_ERR(map); return PTR_ERR(map);
err = bpf_obj_name_cpy(map->name, attr->map_name); err = bpf_obj_name_cpy(map->name, attr->map_name,
if (err) sizeof(attr->map_name));
if (err < 0)
goto free_map; goto free_map;
atomic64_set(&map->refcnt, 1); atomic64_set(&map->refcnt, 1);
...@@ -2098,8 +2100,9 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr) ...@@ -2098,8 +2100,9 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
goto free_prog; goto free_prog;
prog->aux->load_time = ktime_get_boottime_ns(); prog->aux->load_time = ktime_get_boottime_ns();
err = bpf_obj_name_cpy(prog->aux->name, attr->prog_name); err = bpf_obj_name_cpy(prog->aux->name, attr->prog_name,
if (err) sizeof(attr->prog_name));
if (err < 0)
goto free_prog; goto free_prog;
/* run eBPF verifier */ /* run eBPF verifier */
......
...@@ -184,7 +184,6 @@ static int bpf_tcp_ca_init_member(const struct btf_type *t, ...@@ -184,7 +184,6 @@ static int bpf_tcp_ca_init_member(const struct btf_type *t,
{ {
const struct tcp_congestion_ops *utcp_ca; const struct tcp_congestion_ops *utcp_ca;
struct tcp_congestion_ops *tcp_ca; struct tcp_congestion_ops *tcp_ca;
size_t tcp_ca_name_len;
int prog_fd; int prog_fd;
u32 moff; u32 moff;
...@@ -199,13 +198,11 @@ static int bpf_tcp_ca_init_member(const struct btf_type *t, ...@@ -199,13 +198,11 @@ static int bpf_tcp_ca_init_member(const struct btf_type *t,
tcp_ca->flags = utcp_ca->flags; tcp_ca->flags = utcp_ca->flags;
return 1; return 1;
case offsetof(struct tcp_congestion_ops, name): case offsetof(struct tcp_congestion_ops, name):
tcp_ca_name_len = strnlen(utcp_ca->name, sizeof(utcp_ca->name)); if (bpf_obj_name_cpy(tcp_ca->name, utcp_ca->name,
if (!tcp_ca_name_len || sizeof(tcp_ca->name)) <= 0)
tcp_ca_name_len == sizeof(utcp_ca->name))
return -EINVAL; return -EINVAL;
if (tcp_ca_find(utcp_ca->name)) if (tcp_ca_find(utcp_ca->name))
return -EEXIST; return -EEXIST;
memcpy(tcp_ca->name, utcp_ca->name, sizeof(tcp_ca->name));
return 1; return 1;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment