| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1B747351-8336-45FB-918D-5FC3DA18B47D@fb.com>
Date: Thu, 20 May 2021 04:36:46 +0000
From: Song Liu <songliubraving@...com>
To: Dmitrii Banshchikov <me@...que.spb.ru>
CC: "open list:BPF (Safe dynamic programs and tools)"
<bpf@...r.kernel.org>, "ast@...nel.org" <ast@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"daniel@...earbox.net" <daniel@...earbox.net>,
"andrii@...nel.org" <andrii@...nel.org>, Martin Lau <kafai@...com>,
"Yonghong Song" <yhs@...com>,
"john.fastabend@...il.com" <john.fastabend@...il.com>,
"kpsingh@...nel.org" <kpsingh@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Andrey Ignatov <rdna@...com>
Subject: Re: [PATCH bpf-next 07/11] bpfilter: Add struct target
> On May 17, 2021, at 3:53 PM, Dmitrii Banshchikov <me@...que.spb.ru> wrote:
>
> struct target_ops defines polymorphic interface for targets. A target
> consists of pointers to struct target_ops and struct xt_entry_target
> which contains a payload for the target's type.
>
> All target_ops are kept in map target_ops_map by their name.
>
> Signed-off-by: Dmitrii Banshchikov <me@...que.spb.ru>
>
[...]
> index 000000000000..6b65241328da
> --- /dev/null
> +++ b/net/bpfilter/target-ops-map.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2021 Telegram FZ-LLC
> + */
> +
> +#ifndef NET_BPFILTER_TARGET_OPS_MAP_H
> +#define NET_BPFILTER_TARGET_OPS_MAP_H
> +
> +#include "map-common.h"
> +
> +#include <linux/err.h>
> +
> +#include <errno.h>
> +#include <string.h>
> +
> +#include "target.h"
> +
> +struct target_ops_map {
> + struct hsearch_data index;
> +};
Similar to 06/11, target_ops_map seems unnecessary. Also, do we need to
support non-xt targets?
> +
> +static inline int create_target_ops_map(struct target_ops_map *map, size_t nelem)
> +{
> + return create_map(&map->index, nelem);
> +}
> +
> +static inline const struct target_ops *target_ops_map_find(struct target_ops_map *map,
> + const char *name)
> +{
> + const size_t namelen = strnlen(name, BPFILTER_EXTENSION_MAXNAMELEN);
> +
> + if (namelen < BPFILTER_EXTENSION_MAXNAMELEN)
> + return map_find(&map->index, name);
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> +static inline int target_ops_map_insert(struct target_ops_map *map,
> + const struct target_ops *target_ops)
> +{
> + return map_insert(&map->index, target_ops->name, (void *)target_ops);
> +}
> +
> +static inline void free_target_ops_map(struct target_ops_map *map)
> +{
> + free_map(&map->index);
> +}
> +
> +#endif // NET_BPFILTER_TARGET_OPS_MAP_H
> diff --git a/net/bpfilter/target.c b/net/bpfilter/target.c
> new file mode 100644
> index 000000000000..a18fe477f93c
> --- /dev/null
> +++ b/net/bpfilter/target.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2021 Telegram FZ-LLC
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include "target.h"
> +
> +#include <linux/err.h>
> +#include <linux/netfilter/x_tables.h>
> +
> +#include <errno.h>
> +#include <string.h>
> +
> +#include "bflog.h"
> +#include "context.h"
> +#include "target-ops-map.h"
> +
Please add some comments about convert_verdict.
> +static int convert_verdict(int verdict)
> +{
> + return -verdict - 1;
> +}
> +
> +static int standard_target_check(struct context *ctx, const struct bpfilter_ipt_target *ipt_target)
> +{
> + const struct bpfilter_ipt_standard_target *standard_target;
> +
> + standard_target = (const struct bpfilter_ipt_standard_target *)ipt_target;
> +
> + if (standard_target->verdict > 0)
> + return 0;
> +
> + if (standard_target->verdict < 0) {
> + if (standard_target->verdict == BPFILTER_RETURN)
> + return 0;
> +
> + switch (convert_verdict(standard_target->verdict)) {
> + case BPFILTER_NF_ACCEPT:
> + case BPFILTER_NF_DROP:
> + case BPFILTER_NF_QUEUE:
> + return 0;
> + }
> + }
> +
> + BFLOG_DEBUG(ctx, "invalid verdict: %d\n", standard_target->verdict);
> +
> + return -EINVAL;
> +}
> +
> +const struct target_ops standard_target_ops = {
> + .name = "",
> + .revision = 0,
> + .size = sizeof(struct xt_standard_target),
> + .check = standard_target_check,
> +};
> +
> +static int error_target_check(struct context *ctx, const struct bpfilter_ipt_target *ipt_target)
> +{
> + const struct bpfilter_ipt_error_target *error_target;
> + size_t maxlen;
> +
> + error_target = (const struct bpfilter_ipt_error_target *)&ipt_target;
> + maxlen = sizeof(error_target->error_name);
> + if (strnlen(error_target->error_name, maxlen) == maxlen) {
> + BFLOG_DEBUG(ctx, "cannot check error target: too long errorname\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +const struct target_ops error_target_ops = {
> + .name = "ERROR",
> + .revision = 0,
> + .size = sizeof(struct xt_error_target),
> + .check = error_target_check,
> +};
> +
> +int init_target(struct context *ctx, const struct bpfilter_ipt_target *ipt_target,
> + struct target *target)
> +{
> + const size_t maxlen = sizeof(ipt_target->u.user.name);
> + const struct target_ops *found;
> + int err;
> +
> + if (strnlen(ipt_target->u.user.name, maxlen) == maxlen) {
> + BFLOG_DEBUG(ctx, "cannot init target: too long target name\n");
> + return -EINVAL;
> + }
> +
> + found = target_ops_map_find(&ctx->target_ops_map, ipt_target->u.user.name);
> + if (IS_ERR(found)) {
> + BFLOG_DEBUG(ctx, "cannot find target by name: '%s'\n", ipt_target->u.user.name);
> + return PTR_ERR(found);
> + }
> +
> + if (found->size != ipt_target->u.target_size ||
> + found->revision != ipt_target->u.user.revision) {
> + BFLOG_DEBUG(ctx, "invalid target: '%s'\n", ipt_target->u.user.name);
> + return -EINVAL;
> + }
> +
> + err = found->check(ctx, ipt_target);
> + if (err)
> + return err;
> +
> + target->target_ops = found;
> + target->ipt_target = ipt_target;
> +
> + return 0;
> +}
> diff --git a/net/bpfilter/target.h b/net/bpfilter/target.h
> new file mode 100644
> index 000000000000..5d9c4c459c05
> --- /dev/null
> +++ b/net/bpfilter/target.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2021 Telegram FZ-LLC
> + */
> +
> +#ifndef NET_BPFILTER_TARGET_H
> +#define NET_BPFILTER_TARGET_H
> +
> +#include "../../include/uapi/linux/bpfilter.h"
> +
> +#include <stdint.h>
> +
> +struct context;
> +struct target_ops_map;
> +
> +struct target_ops {
> + char name[BPFILTER_EXTENSION_MAXNAMELEN];
> + uint16_t size;
Mis-aligned "size".
> + uint8_t revision;
> + int (*check)(struct context *ctx, const struct bpfilter_ipt_target *ipt_target);
> +};
> +
> +struct target {
> + const struct target_ops *target_ops;
> + const struct bpfilter_ipt_target *ipt_target;
> +};
> +
> +extern const struct target_ops standard_target_ops;
> +extern const struct target_ops error_target_ops;
> +
> +int init_target(struct context *ctx, const struct bpfilter_ipt_target *ipt_target,
> + struct target *target);
> +
> +#endif // NET_BPFILTER_TARGET_H
> diff --git a/tools/testing/selftests/bpf/bpfilter/.gitignore b/tools/testing/selftests/bpf/bpfilter/.gitignore
> index e5073231f811..1856d0515f49 100644
> --- a/tools/testing/selftests/bpf/bpfilter/.gitignore
> +++ b/tools/testing/selftests/bpf/bpfilter/.gitignore
> @@ -2,3 +2,4 @@
> test_io
> test_map
> test_match
> +test_target
> diff --git a/tools/testing/selftests/bpf/bpfilter/Makefile b/tools/testing/selftests/bpf/bpfilter/Makefile
> index 362c9a28b88d..78da74b9ee68 100644
> --- a/tools/testing/selftests/bpf/bpfilter/Makefile
> +++ b/tools/testing/selftests/bpf/bpfilter/Makefile
> @@ -11,6 +11,7 @@ CFLAGS += -Wall -g -pthread -I$(TOOLSINCDIR) -I$(APIDIR) -I$(BPFILTERSRCDIR)
> TEST_GEN_PROGS += test_io
> TEST_GEN_PROGS += test_map
> TEST_GEN_PROGS += test_match
> +TEST_GEN_PROGS += test_target
>
> KSFT_KHDR_INSTALL := 1
>
> @@ -19,4 +20,6 @@ include ../../lib.mk
> $(OUTPUT)/test_io: test_io.c $(BPFILTERSRCDIR)/io.c
> $(OUTPUT)/test_map: test_map.c $(BPFILTERSRCDIR)/map-common.c
> $(OUTPUT)/test_match: test_match.c $(BPFILTERSRCDIR)/match.c $(BPFILTERSRCDIR)/map-common.c \
> - $(BPFILTERSRCDIR)/context.c $(BPFILTERSRCDIR)/bflog.c
> + $(BPFILTERSRCDIR)/context.c $(BPFILTERSRCDIR)/bflog.c $(BPFILTERSRCDIR)/target.c
> +$(OUTPUT)/test_target: test_target.c $(BPFILTERSRCDIR)/target.c $(BPFILTERSRCDIR)/map-common.c \
> + $(BPFILTERSRCDIR)/context.c $(BPFILTERSRCDIR)/bflog.c $(BPFILTERSRCDIR)/match.c
> diff --git a/tools/testing/selftests/bpf/bpfilter/bpfilter_util.h b/tools/testing/selftests/bpf/bpfilter/bpfilter_util.h
> new file mode 100644
> index 000000000000..d82ff86f280e
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/bpfilter/bpfilter_util.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef BPFILTER_UTIL_H
> +#define BPFILTER_UTIL_H
> +
> +#include <linux/bpfilter.h>
> +#include <linux/netfilter/x_tables.h>
> +
> +#include <stdio.h>
> +
> +static inline void init_standard_target(struct xt_standard_target *ipt_target, int revision,
> + int verdict)
> +{
> + snprintf(ipt_target->target.u.user.name, sizeof(ipt_target->target.u.user.name), "%s",
> + BPFILTER_STANDARD_TARGET);
> + ipt_target->target.u.user.revision = revision;
> + ipt_target->target.u.user.target_size = sizeof(*ipt_target);
> + ipt_target->verdict = verdict;
> +}
> +
> +static inline void init_error_target(struct xt_error_target *ipt_target, int revision,
> + const char *error_name)
> +{
> + snprintf(ipt_target->target.u.user.name, sizeof(ipt_target->target.u.user.name), "%s",
> + BPFILTER_ERROR_TARGET);
> + ipt_target->target.u.user.revision = revision;
> + ipt_target->target.u.user.target_size = sizeof(*ipt_target);
> + snprintf(ipt_target->errorname, sizeof(ipt_target->errorname), "%s", error_name);
> +}
> +
> +#endif // BPFILTER_UTIL_H
>
[...]
>
Powered by blists - more mailing lists