Currently, it's still quite hard to figure out if a prog passed the verifier, but later gets rejected due to different tail call ownership. Figure out whether that is the case and provide appropriate error messages to the user.
Signed-off-by: Daniel Borkmann <dan...@iogearbox.net> --- lib/bpf.c | 226 ++++++++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 169 insertions(+), 57 deletions(-) diff --git a/lib/bpf.c b/lib/bpf.c index 7eb5cd9..5d9f0a5 100644 --- a/lib/bpf.c +++ b/lib/bpf.c @@ -344,15 +344,24 @@ static void bpf_map_pin_report(const struct bpf_elf_map *pin, fprintf(stderr, "\n"); } -static int bpf_map_selfcheck_pinned(int fd, const struct bpf_elf_map *map, - int length, enum bpf_prog_type type) +struct bpf_prog_data { + unsigned int type; + unsigned int jited; +}; + +struct bpf_map_ext { + struct bpf_prog_data owner; +}; + +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]; - struct bpf_elf_map tmp = {}, zero = {}; - unsigned int val, owner_type = 0; FILE *fp; snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd); + memset(map, 0, sizeof(*map)); fp = fopen(file, "r"); if (!fp) { @@ -362,27 +371,48 @@ static int bpf_map_selfcheck_pinned(int fd, const struct bpf_elf_map *map, while (fgets(buff, sizeof(buff), fp)) { if (sscanf(buff, "map_type:\t%u", &val) == 1) - tmp.type = val; + map->type = val; else if (sscanf(buff, "key_size:\t%u", &val) == 1) - tmp.size_key = val; + map->size_key = val; else if (sscanf(buff, "value_size:\t%u", &val) == 1) - tmp.size_value = val; + map->size_value = val; else if (sscanf(buff, "max_entries:\t%u", &val) == 1) - tmp.max_elem = val; + map->max_elem = val; else if (sscanf(buff, "map_flags:\t%i", &val) == 1) - tmp.flags = val; + map->flags = val; else if (sscanf(buff, "owner_prog_type:\t%i", &val) == 1) owner_type = val; + else if (sscanf(buff, "owner_jited:\t%i", &val) == 1) + owner_jited = val; } fclose(fp); + if (ext) { + memset(ext, 0, sizeof(*ext)); + ext->owner.type = owner_type; + ext->owner.jited = owner_jited; + } + + return 0; +} + +static int bpf_map_selfcheck_pinned(int fd, const struct bpf_elf_map *map, + struct bpf_map_ext *ext, int length, + enum bpf_prog_type type) +{ + struct bpf_elf_map tmp, zero = {}; + int ret; + + ret = bpf_derive_elf_map_from_fdinfo(fd, &tmp, ext); + if (ret < 0) + return ret; /* The decision to reject this is on kernel side eventually, but * at least give the user a chance to know what's wrong. */ - if (owner_type && owner_type != type) + if (ext->owner.type && ext->owner.type != type) fprintf(stderr, "Program array map owner types differ: %u (obj) != %u (pin)\n", - type, owner_type); + type, ext->owner.type); if (!memcmp(&tmp, map, length)) { return 0; @@ -882,6 +912,7 @@ int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv) .argc = argc, .argv = argv, }; + struct bpf_map_ext ext = {}; int ret, prog_fd, map_fd; enum bpf_mode mode; uint32_t map_key; @@ -908,7 +939,7 @@ int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv) goto out_prog; } - ret = bpf_map_selfcheck_pinned(map_fd, &test, + ret = bpf_map_selfcheck_pinned(map_fd, &test, &ext, offsetof(struct bpf_elf_map, max_elem), type); if (ret < 0) { @@ -981,7 +1012,12 @@ struct bpf_hash_entry { struct bpf_hash_entry *next; }; +struct bpf_config { + unsigned int jit_enabled; +}; + struct bpf_elf_ctx { + struct bpf_config cfg; Elf *elf_fd; GElf_Ehdr elf_hdr; Elf_Data *sym_tab; @@ -989,6 +1025,7 @@ struct bpf_elf_ctx { int obj_fd; int map_fds[ELF_MAX_MAPS]; struct bpf_elf_map maps[ELF_MAX_MAPS]; + struct bpf_map_ext maps_ext[ELF_MAX_MAPS]; int sym_num; int map_num; int map_len; @@ -1425,39 +1462,6 @@ static int bpf_find_map_id(const struct bpf_elf_ctx *ctx, uint32_t id) return -ENOENT; } -static int bpf_derive_elf_map_from_fdinfo(int fd, struct bpf_elf_map *map) -{ - char file[PATH_MAX], buff[4096]; - unsigned int val; - FILE *fp; - - snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd); - - memset(map, 0, sizeof(*map)); - - fp = fopen(file, "r"); - if (!fp) { - fprintf(stderr, "No procfs support?!\n"); - return -EIO; - } - - while (fgets(buff, sizeof(buff), fp)) { - if (sscanf(buff, "map_type:\t%u", &val) == 1) - map->type = val; - else if (sscanf(buff, "key_size:\t%u", &val) == 1) - map->size_key = val; - else if (sscanf(buff, "value_size:\t%u", &val) == 1) - map->size_value = val; - else if (sscanf(buff, "max_entries:\t%u", &val) == 1) - map->max_elem = val; - else if (sscanf(buff, "map_flags:\t%i", &val) == 1) - map->flags = val; - } - - fclose(fp); - return 0; -} - static void bpf_report_map_in_map(int outer_fd, int inner_fd, uint32_t idx) { struct bpf_elf_map outer_map; @@ -1465,7 +1469,7 @@ static void bpf_report_map_in_map(int outer_fd, int inner_fd, uint32_t idx) fprintf(stderr, "Cannot insert map into map! "); - ret = bpf_derive_elf_map_from_fdinfo(outer_fd, &outer_map); + ret = bpf_derive_elf_map_from_fdinfo(outer_fd, &outer_map, NULL); if (!ret) { if (idx >= outer_map.max_elem && outer_map.type == BPF_MAP_TYPE_ARRAY_OF_MAPS) { @@ -1484,14 +1488,15 @@ static bool bpf_is_map_in_map_type(const struct bpf_elf_map *map) map->type == BPF_MAP_TYPE_HASH_OF_MAPS; } -static int bpf_map_attach(const char *name, const struct bpf_elf_map *map, - struct bpf_elf_ctx *ctx, int *have_map_in_map) +static int bpf_map_attach(const char *name, struct bpf_elf_ctx *ctx, + const struct bpf_elf_map *map, struct bpf_map_ext *ext, + int *have_map_in_map) { int fd, ret, map_inner_fd = 0; fd = bpf_probe_pinned(name, ctx, map->pinning); if (fd > 0) { - ret = bpf_map_selfcheck_pinned(fd, map, + ret = bpf_map_selfcheck_pinned(fd, map, ext, offsetof(struct bpf_elf_map, id), ctx->type); if (ret < 0) { @@ -1581,8 +1586,8 @@ static int bpf_maps_attach_all(struct bpf_elf_ctx *ctx) if (!map_name) return -EIO; - fd = bpf_map_attach(map_name, &ctx->maps[i], ctx, - &have_map_in_map); + fd = bpf_map_attach(map_name, ctx, &ctx->maps[i], + &ctx->maps_ext[i], &have_map_in_map); if (fd < 0) return fd; @@ -1597,8 +1602,8 @@ static int bpf_maps_attach_all(struct bpf_elf_ctx *ctx) if (!map_name) return -EIO; - fd = bpf_map_attach(map_name, &ctx->maps[i], ctx, - NULL); + fd = bpf_map_attach(map_name, ctx, &ctx->maps[i], + &ctx->maps_ext[i], NULL); if (fd < 0) return fd; @@ -1901,9 +1906,15 @@ static int bpf_fetch_prog(struct bpf_elf_ctx *ctx, const char *section, return fd; } +struct bpf_tail_call_props { + unsigned int total; + unsigned int jited; +}; + static int bpf_apply_relo_data(struct bpf_elf_ctx *ctx, struct bpf_elf_sec_data *data_relo, - struct bpf_elf_sec_data *data_insn) + struct bpf_elf_sec_data *data_insn, + struct bpf_tail_call_props *props) { Elf_Data *idata = data_insn->sec_data; GElf_Shdr *rhdr = &data_relo->sec_hdr; @@ -1943,6 +1954,13 @@ static int bpf_apply_relo_data(struct bpf_elf_ctx *ctx, return -EINVAL; if (!ctx->map_fds[rmap]) return -EINVAL; + if (ctx->maps[rmap].type == BPF_MAP_TYPE_PROG_ARRAY) { + props->total++; + if (ctx->maps_ext[rmap].owner.jited || + (ctx->maps_ext[rmap].owner.type == 0 && + ctx->cfg.jit_enabled)) + props->jited++; + } if (ctx->verbose) fprintf(stderr, "Map \'%s\' (%d) injected into prog section \'%s\' at offset %u!\n", @@ -1964,6 +1982,8 @@ static int bpf_fetch_prog_relo(struct bpf_elf_ctx *ctx, const char *section, int ret, idx, i, fd = -1; for (i = 1; i < ctx->elf_hdr.e_shnum; i++) { + struct bpf_tail_call_props props = {}; + ret = bpf_fill_section_data(ctx, i, &data_relo); if (ret < 0 || data_relo.sec_hdr.sh_type != SHT_REL) continue; @@ -1979,7 +1999,7 @@ static int bpf_fetch_prog_relo(struct bpf_elf_ctx *ctx, const char *section, *sseen = true; - ret = bpf_apply_relo_data(ctx, &data_relo, &data_insn); + ret = bpf_apply_relo_data(ctx, &data_relo, &data_insn, &props); if (ret < 0) { *lderr = true; return ret; @@ -1994,6 +2014,16 @@ static int bpf_fetch_prog_relo(struct bpf_elf_ctx *ctx, const char *section, fd = bpf_prog_attach(section, &prog, ctx); if (fd < 0) { *lderr = true; + if (props.total) { + if (ctx->cfg.jit_enabled && + props.total != props.jited) + fprintf(stderr, "JIT enabled, but only %u/%u tail call maps in the program have JITed owner!\n", + props.jited, props.total); + if (!ctx->cfg.jit_enabled && + props.jited) + fprintf(stderr, "JIT disabled, but %u/%u tail call maps in the program have JITed owner!\n", + props.jited, props.total); + } return fd; } @@ -2031,6 +2061,51 @@ static int bpf_find_map_by_id(struct bpf_elf_ctx *ctx, uint32_t id) return -1; } +struct bpf_jited_aux { + int prog_fd; + int map_fd; + struct bpf_prog_data prog; + struct bpf_map_ext map; +}; + +static int bpf_derive_prog_from_fdinfo(int fd, struct bpf_prog_data *prog) +{ + char file[PATH_MAX], buff[4096]; + unsigned int val; + FILE *fp; + + snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd); + memset(prog, 0, sizeof(*prog)); + + fp = fopen(file, "r"); + if (!fp) { + fprintf(stderr, "No procfs support?!\n"); + return -EIO; + } + + while (fgets(buff, sizeof(buff), fp)) { + if (sscanf(buff, "prog_type:\t%u", &val) == 1) + prog->type = val; + else if (sscanf(buff, "prog_jited:\t%u", &val) == 1) + prog->jited = val; + } + + fclose(fp); + return 0; +} + +static int bpf_tail_call_get_aux(struct bpf_jited_aux *aux) +{ + struct bpf_elf_map tmp; + int ret; + + ret = bpf_derive_elf_map_from_fdinfo(aux->map_fd, &tmp, &aux->map); + if (!ret) + ret = bpf_derive_prog_from_fdinfo(aux->prog_fd, &aux->prog); + + return ret; +} + static int bpf_fill_prog_arrays(struct bpf_elf_ctx *ctx) { struct bpf_elf_sec_data data; @@ -2060,10 +2135,31 @@ static int bpf_fill_prog_arrays(struct bpf_elf_ctx *ctx) ret = bpf_map_update(ctx->map_fds[idx], &key_id, &fd, BPF_ANY); if (ret < 0) { - if (errno == E2BIG) + struct bpf_jited_aux aux = {}; + + ret = -errno; + if (errno == E2BIG) { fprintf(stderr, "Tail call key %u for map %u out of bounds?\n", key_id, map_id); - return -errno; + return ret; + } + + aux.map_fd = ctx->map_fds[idx]; + aux.prog_fd = fd; + + if (bpf_tail_call_get_aux(&aux)) + return ret; + if (!aux.map.owner.type) + return ret; + + if (aux.prog.type != aux.map.owner.type) + fprintf(stderr, "Tail call map owned by prog type %u, but prog type is %u!\n", + aux.map.owner.type, aux.prog.type); + if (aux.prog.jited != aux.map.owner.jited) + fprintf(stderr, "Tail call map %s jited, but prog %s!\n", + aux.map.owner.jited ? "is" : "not", + aux.prog.jited ? "is" : "not"); + return ret; } ctx->sec_done[i] = true; @@ -2221,6 +2317,21 @@ static int bpf_elf_check_ehdr(const struct bpf_elf_ctx *ctx) return 0; } +static void bpf_get_cfg(struct bpf_elf_ctx *ctx) +{ + static const char *path_jit = "/proc/sys/net/core/bpf_jit_enable"; + int fd; + + fd = open(path_jit, O_RDONLY); + if (fd > 0) { + char tmp[16] = {}; + + if (read(fd, tmp, sizeof(tmp)) > 0) + ctx->cfg.jit_enabled = atoi(tmp); + close(fd); + } +} + static int bpf_elf_ctx_init(struct bpf_elf_ctx *ctx, const char *pathname, enum bpf_prog_type type, bool verbose) { @@ -2231,6 +2342,7 @@ static int bpf_elf_ctx_init(struct bpf_elf_ctx *ctx, const char *pathname, return ret; memset(ctx, 0, sizeof(*ctx)); + bpf_get_cfg(ctx); ctx->verbose = verbose; ctx->type = type; -- 2.9.3