* Re: [PATCH net v2] ipv4: don't set IPv6 only flags to IPv4 addresses
From: David Ahern @ 2019-07-01 17:19 UTC (permalink / raw)
To: Matteo Croce, netdev
Cc: linux-kernel, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI
In-Reply-To: <20190701170155.1967-1-mcroce@redhat.com>
On 7/1/19 11:01 AM, Matteo Croce wrote:
> Avoid the situation where an IPV6 only flag is applied to an IPv4 address:
>
> # ip addr add 192.0.2.1/24 dev dummy0 nodad home mngtmpaddr noprefixroute
> # ip -4 addr show dev dummy0
> 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> inet 192.0.2.1/24 scope global noprefixroute dummy0
> valid_lft forever preferred_lft forever
>
> Or worse, by sending a malicious netlink command:
>
> # ip -4 addr show dev dummy0
> 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> inet 192.0.2.1/24 scope global nodad optimistic dadfailed home tentative mngtmpaddr noprefixroute stable-privacy dummy0
> valid_lft forever preferred_lft forever
>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
> ---
> net/ipv4/devinet.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index c6bd0f7a020a..c5ebfa199794 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -62,6 +62,11 @@
> #include <net/net_namespace.h>
> #include <net/addrconf.h>
>
> +#define IPV6ONLY_FLAGS \
> + (IFA_F_NODAD | IFA_F_OPTIMISTIC | IFA_F_DADFAILED | \
> + IFA_F_HOMEADDRESS | IFA_F_TENTATIVE | \
> + IFA_F_MANAGETEMPADDR | IFA_F_STABLE_PRIVACY)
> +
> static struct ipv4_devconf ipv4_devconf = {
> .data = {
> [IPV4_DEVCONF_ACCEPT_REDIRECTS - 1] = 1,
> @@ -468,6 +473,9 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
> ifa->ifa_flags &= ~IFA_F_SECONDARY;
> last_primary = &in_dev->ifa_list;
>
> + /* Don't set IPv6 only flags to IPv4 addresses */
> + ifa->ifa_flags &= ~IPV6ONLY_FLAGS;
> +
> for (ifap = &in_dev->ifa_list; (ifa1 = *ifap) != NULL;
> ifap = &ifa1->ifa_next) {
> if (!(ifa1->ifa_flags & IFA_F_SECONDARY) &&
>
I guess at this point we can fail the address add, so this is the best
option. rtm_to_ifaddr could set a message in extack about invalid flags
- not fail the change, just warn the user that flags will be ignored.
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH v4 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests
From: Yonghong Song @ 2019-07-01 17:18 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-9-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
> expected.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> .../selftests/bpf/prog_tests/attach_probe.c | 155 ++++++++++++++++++
> .../selftests/bpf/progs/test_attach_probe.c | 55 +++++++
> 2 files changed, 210 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> new file mode 100644
> index 000000000000..f22929063c58
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +
> +ssize_t get_base_addr() {
> + size_t start;
> + char buf[256];
> + FILE *f;
> +
> + f = fopen("/proc/self/maps", "r");
> + if (!f)
> + return -errno;
> +
> + while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
> + if (strcmp(buf, "r-xp") == 0) {
> + fclose(f);
> + return start;
> + }
> + }
> +
> + fclose(f);
> + return -EINVAL;
> +}
> +
> +void test_attach_probe(void)
> +{
> + const char *kprobe_name = "kprobe/sys_nanosleep";
> + const char *kretprobe_name = "kretprobe/sys_nanosleep";
> + const char *uprobe_name = "uprobe/trigger_func";
> + const char *uretprobe_name = "uretprobe/trigger_func";
> + const int kprobe_idx = 0, kretprobe_idx = 1;
> + const int uprobe_idx = 2, uretprobe_idx = 3;
> + const char *file = "./test_attach_probe.o";
> + struct bpf_program *kprobe_prog, *kretprobe_prog;
> + struct bpf_program *uprobe_prog, *uretprobe_prog;
> + struct bpf_object *obj;
> + int err, prog_fd, duration = 0, res;
> + struct bpf_link *kprobe_link = NULL;
> + struct bpf_link *kretprobe_link = NULL;
> + struct bpf_link *uprobe_link = NULL;
> + struct bpf_link *uretprobe_link = NULL;
> + int results_map_fd;
> + size_t uprobe_offset;
> + ssize_t base_addr;
> +
> + base_addr = get_base_addr();
> + if (CHECK(base_addr < 0, "get_base_addr",
> + "failed to find base addr: %zd", base_addr))
> + return;
> + uprobe_offset = (size_t)&get_base_addr - base_addr;
> +
> + /* load programs */
> + err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> + if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> + return;
> +
> + kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
> + if (CHECK(!kprobe_prog, "find_probe",
> + "prog '%s' not found\n", kprobe_name))
> + goto cleanup;
> + kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
> + if (CHECK(!kretprobe_prog, "find_probe",
> + "prog '%s' not found\n", kretprobe_name))
> + goto cleanup;
> + uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
> + if (CHECK(!uprobe_prog, "find_probe",
> + "prog '%s' not found\n", uprobe_name))
> + goto cleanup;
> + uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
> + if (CHECK(!uretprobe_prog, "find_probe",
> + "prog '%s' not found\n", uretprobe_name))
> + goto cleanup;
> +
> + /* load maps */
> + results_map_fd = bpf_find_map(__func__, obj, "results_map");
> + if (CHECK(results_map_fd < 0, "find_results_map",
> + "err %d\n", results_map_fd))
> + goto cleanup;
> +
> + kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
> + false /* retprobe */,
> + "sys_nanosleep");
> + if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
> + "err %ld\n", PTR_ERR(kprobe_link)))
> + goto cleanup;
> +
> + kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
> + true /* retprobe */,
> + "sys_nanosleep");
> + if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
> + "err %ld\n", PTR_ERR(kretprobe_link)))
> + goto cleanup;
> +
> + uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
> + false /* retprobe */,
> + 0 /* self pid */,
> + "/proc/self/exe",
> + uprobe_offset);
> + if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
> + "err %ld\n", PTR_ERR(uprobe_link)))
> + goto cleanup;
> +
> + uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
> + true /* retprobe */,
> + -1 /* any pid */,
> + "/proc/self/exe",
> + uprobe_offset);
> + if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
> + "err %ld\n", PTR_ERR(uretprobe_link)))
> + goto cleanup;
> +
> + /* trigger & validate kprobe && kretprobe */
> + usleep(1);
> +
> + err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
> + if (CHECK(err, "get_kprobe_res",
> + "failed to get kprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
> + "wrong kprobe res: %d\n", res))
> + goto cleanup;
> +
> + err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
> + if (CHECK(err, "get_kretprobe_res",
> + "failed to get kretprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
> + "wrong kretprobe res: %d\n", res))
> + goto cleanup;
> +
> + /* trigger & validate uprobe & uretprobe */
> + get_base_addr();
> +
> + err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
> + if (CHECK(err, "get_uprobe_res",
> + "failed to get uprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
> + "wrong uprobe res: %d\n", res))
> + goto cleanup;
> +
> + err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
> + if (CHECK(err, "get_uretprobe_res",
> + "failed to get uretprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
> + "wrong uretprobe res: %d\n", res))
> + goto cleanup;
> +
> +cleanup:
> + bpf_link__destroy(kprobe_link);
> + bpf_link__destroy(kretprobe_link);
> + bpf_link__destroy(uprobe_link);
> + bpf_link__destroy(uretprobe_link);
if any error happens, kprobe_link etc. will be a non-NULL pointer
indicating an error. the above bpf_link__destroy() won't work
properly. The same for patch #9.
> + bpf_object__close(obj);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> new file mode 100644
> index 000000000000..7a7c5cd728c8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2017 Facebook
> +
> +#include <linux/ptrace.h>
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +struct {
> + int type;
> + int max_entries;
> + int *key;
> + int *value;
> +} results_map SEC(".maps") = {
> + .type = BPF_MAP_TYPE_ARRAY,
> + .max_entries = 4,
> +};
> +
> +SEC("kprobe/sys_nanosleep")
> +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
> +{
> + const int key = 0, value = 1;
> +
> + bpf_map_update_elem(&results_map, &key, &value, 0);
> + return 0;
> +}
> +
> +SEC("kretprobe/sys_nanosleep")
> +int handle_sys_getpid_return(struct pt_regs *ctx)
> +{
> + const int key = 1, value = 2;
> +
> + bpf_map_update_elem(&results_map, &key, &value, 0);
> + return 0;
> +}
> +
> +SEC("uprobe/trigger_func")
> +int handle_uprobe_entry(struct pt_regs *ctx)
> +{
> + const int key = 2, value = 3;
> +
> + bpf_map_update_elem(&results_map, &key, &value, 0);
> + return 0;
> +}
> +
> +SEC("uretprobe/trigger_func")
> +int handle_uprobe_return(struct pt_regs *ctx)
> +{
> + const int key = 3, value = 4;
> +
> + bpf_map_update_elem(&results_map, &key, &value, 0);
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;
>
^ permalink raw reply
* Re: [PATCH v4 bpf-next 7/9] selftests/bpf: switch test to new attach_perf_event API
From: Yonghong Song @ 2019-07-01 17:16 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-8-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Use new bpf_program__attach_perf_event() in test previously relying on
> direct ioctl manipulations.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> .../bpf/prog_tests/stacktrace_build_id_nmi.c | 31 +++++++++----------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> index 1c1a2f75f3d8..9557b7dfb782 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> @@ -17,6 +17,7 @@ static __u64 read_perf_max_sample_freq(void)
> void test_stacktrace_build_id_nmi(void)
> {
> int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
> + const char *prog_name = "tracepoint/random/urandom_read";
> const char *file = "./test_stacktrace_build_id.o";
> int err, pmu_fd, prog_fd;
> struct perf_event_attr attr = {
> @@ -25,7 +26,9 @@ void test_stacktrace_build_id_nmi(void)
> .config = PERF_COUNT_HW_CPU_CYCLES,
> };
> __u32 key, previous_key, val, duration = 0;
> + struct bpf_program *prog;
> struct bpf_object *obj;
> + struct bpf_link *link;
> char buf[256];
> int i, j;
> struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
> @@ -39,6 +42,10 @@ void test_stacktrace_build_id_nmi(void)
> if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
> return;
>
> + prog = bpf_object__find_program_by_title(obj, prog_name);
> + if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
> + goto close_prog;
> +
> pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
> 0 /* cpu 0 */, -1 /* group id */,
> 0 /* flags */);
> @@ -47,15 +54,12 @@ void test_stacktrace_build_id_nmi(void)
> pmu_fd, errno))
> goto close_prog;
>
> - err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
> - if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
> - err, errno))
> - goto close_pmu;
> -
> - err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
> - if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
> - err, errno))
> - goto disable_pmu;
> + link = bpf_program__attach_perf_event(prog, pmu_fd);
> + if (CHECK(IS_ERR(link), "attach_perf_event",
> + "err %ld\n", PTR_ERR(link))) {
> + close(pmu_fd);
> + goto close_prog;
> + }
>
> /* find map fds */
> control_map_fd = bpf_find_map(__func__, obj, "control_map");
> @@ -134,8 +138,7 @@ void test_stacktrace_build_id_nmi(void)
> * try it one more time.
> */
> if (build_id_matches < 1 && retry--) {
> - ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
> - close(pmu_fd);
> + bpf_link__destroy(link);
> bpf_object__close(obj);
> printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
> __func__);
> @@ -154,11 +157,7 @@ void test_stacktrace_build_id_nmi(void)
> */
>
> disable_pmu:
> - ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
> -
> -close_pmu:
> - close(pmu_fd);
> -
> + bpf_link__destroy(link);
There is a problem in bpf_link__destroy(link).
The "link = bpf_program__attach_perf_event(prog, pmu_fd)"
may be an error pointer (IS_ERR(link) is true), in which
case, link should be reset to NULL and then call
bpf_link__destroy(link). Otherwise, the program may
segfault or function incorrectly.
> close_prog:
> bpf_object__close(obj);
> }
>
^ permalink raw reply
* Re: [PATCH v4 bpf-next 6/9] libbpf: add raw tracepoint attach API
From: Yonghong Song @ 2019-07-01 17:13 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-7-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add a wrapper utilizing bpf_link "infrastructure" to allow attaching BPF
> programs to raw tracepoints.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 37 +++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 3 +++
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 41 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8ad4f915df38..f8c7a7ecb35e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4263,6 +4263,43 @@ struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
> return link;
> }
>
> +static int bpf_link__destroy_fd(struct bpf_link *link)
> +{
> + struct bpf_link_fd *l = (void *)link;
> +
> + return close(l->fd);
> +}
> +
> +struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> + const char *tp_name)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link_fd *link;
> + int prog_fd, pfd;
> +
> + prog_fd = bpf_program__fd(prog);
> + if (prog_fd < 0) {
> + pr_warning("program '%s': can't attach before loaded\n",
> + bpf_program__title(prog, false));
> + return ERR_PTR(-EINVAL);
> + }
> +
> + link = malloc(sizeof(*link));
> + link->link.destroy = &bpf_link__destroy_fd;
You can move the "link = malloc(...)" etc. after
bpf_raw_tracepoint_open(). That way, you do not need to free(link)
in the error case.
> +
> + pfd = bpf_raw_tracepoint_open(tp_name, prog_fd);
> + if (pfd < 0) {
> + pfd = -errno;
> + free(link);
> + pr_warning("program '%s': failed to attach to raw tracepoint '%s': %s\n",
> + bpf_program__title(prog, false), tp_name,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(pfd);
> + }
> + link->fd = pfd;
> + return (struct bpf_link *)link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 60611f4b4e1d..f55933784f95 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -182,6 +182,9 @@ LIBBPF_API struct bpf_link *
> bpf_program__attach_tracepoint(struct bpf_program *prog,
> const char *tp_category,
> const char *tp_name);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> + const char *tp_name);
>
> struct bpf_insn;
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 3c618b75ef65..e6b7d4edbc93 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
> bpf_object__load_xattr;
> bpf_program__attach_kprobe;
> bpf_program__attach_perf_event;
> + bpf_program__attach_raw_tracepoint;
> bpf_program__attach_tracepoint;
> bpf_program__attach_uprobe;
> btf_dump__dump_type;
>
^ permalink raw reply
* Re: [PATCH v4 bpf-next 5/9] libbpf: add tracepoint attach API
From: Yonghong Song @ 2019-07-01 17:10 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-6-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Allow attaching BPF programs to kernel tracepoint BPF hooks specified by
> category and name.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 79 ++++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 4 ++
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 84 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2f79e9563db9..8ad4f915df38 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4184,6 +4184,85 @@ struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
> return link;
> }
>
> +static int determine_tracepoint_id(const char *tp_category,
> + const char *tp_name)
> +{
> + char file[PATH_MAX];
> + int ret;
> +
> + ret = snprintf(file, sizeof(file),
> + "/sys/kernel/debug/tracing/events/%s/%s/id",
> + tp_category, tp_name);
> + if (ret < 0)
> + return -errno;
> + if (ret >= sizeof(file)) {
> + pr_debug("tracepoint %s/%s path is too long\n",
> + tp_category, tp_name);
> + return -E2BIG;
> + }
> + return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int perf_event_open_tracepoint(const char *tp_category,
> + const char *tp_name)
> +{
> + struct perf_event_attr attr = {};
> + char errmsg[STRERR_BUFSIZE];
> + int tp_id, pfd, err;
> +
> + tp_id = determine_tracepoint_id(tp_category, tp_name);
> + if (tp_id < 0) {
> + pr_warning("failed to determine tracepoint '%s/%s' perf ID: %s\n",
nit: "perf ID" is not accurate. Maybe "event ID" or "perf event ID"?
> + tp_category, tp_name,
> + libbpf_strerror_r(tp_id, errmsg, sizeof(errmsg)));
> + return tp_id;
> + }
> +
> + attr.type = PERF_TYPE_TRACEPOINT;
> + attr.size = sizeof(attr);
> + attr.config = tp_id;
> +
> + pfd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, 0 /* cpu */,
> + -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> + if (pfd < 0) {
> + err = -errno;
> + pr_warning("tracepoint '%s/%s' perf_event_open() failed: %s\n",
> + tp_category, tp_name,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return err;
> + }
> + return pfd;
> +}
> +
> +struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
> + const char *tp_category,
> + const char *tp_name)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link *link;
> + int pfd, err;
> +
> + pfd = perf_event_open_tracepoint(tp_category, tp_name);
> + if (pfd < 0) {
> + pr_warning("program '%s': failed to create tracepoint '%s/%s' perf event: %s\n",
> + bpf_program__title(prog, false),
> + tp_category, tp_name,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(pfd);
> + }
> + link = bpf_program__attach_perf_event(prog, pfd);
> + if (IS_ERR(link)) {
> + close(pfd);
> + err = PTR_ERR(link);
> + pr_warning("program '%s': failed to attach to tracepoint '%s/%s': %s\n",
> + bpf_program__title(prog, false),
> + tp_category, tp_name,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return link;
> + }
> + return link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index bd767cc11967..60611f4b4e1d 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -178,6 +178,10 @@ LIBBPF_API struct bpf_link *
> bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> pid_t pid, const char *binary_path,
> size_t func_offset);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_tracepoint(struct bpf_program *prog,
> + const char *tp_category,
> + const char *tp_name);
>
> struct bpf_insn;
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 57a40fb60718..3c618b75ef65 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
> bpf_object__load_xattr;
> bpf_program__attach_kprobe;
> bpf_program__attach_perf_event;
> + bpf_program__attach_tracepoint;
> bpf_program__attach_uprobe;
> btf_dump__dump_type;
> btf_dump__free;
>
^ permalink raw reply
* [PATCH net-next 5/5] ipv4: use indirect call wrappers for {tcp,udp}_{recv,send}msg()
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
This avoids an indirect call per syscall for common ipv4 transports
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv4/af_inet.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 8421e2f5bbb3..9a2f17d0c5f5 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -797,6 +797,8 @@ int inet_send_prepare(struct sock *sk)
}
EXPORT_SYMBOL_GPL(inet_send_prepare);
+INDIRECT_CALLABLE_DECLARE(int udp_sendmsg(struct sock *, struct msghdr *,
+ size_t));
int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct sock *sk = sock->sk;
@@ -804,7 +806,8 @@ int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (unlikely(inet_send_prepare(sk)))
return -EAGAIN;
- return sk->sk_prot->sendmsg(sk, msg, size);
+ return INDIRECT_CALL_2(sk->sk_prot->sendmsg, tcp_sendmsg, udp_sendmsg,
+ sk, msg, size);
}
EXPORT_SYMBOL(inet_sendmsg);
@@ -822,6 +825,8 @@ ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
}
EXPORT_SYMBOL(inet_sendpage);
+INDIRECT_CALLABLE_DECLARE(int udp_recvmsg(struct sock *, struct msghdr *,
+ size_t, int, int, int *));
int inet_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
int flags)
{
@@ -832,8 +837,9 @@ int inet_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
if (likely(!(flags & MSG_ERRQUEUE)))
sock_rps_record_flow(sk);
- err = sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
- flags & ~MSG_DONTWAIT, &addr_len);
+ err = INDIRECT_CALL_2(sk->sk_prot->recvmsg, tcp_recvmsg, udp_recvmsg,
+ sk, msg, size, flags & MSG_DONTWAIT,
+ flags & ~MSG_DONTWAIT, &addr_len);
if (err >= 0)
msg->msg_namelen = addr_len;
return err;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 4/5] ipv6: use indirect call wrappers for {tcp,udpv6}_{recv,send}msg()
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
This avoids an indirect call per syscall for common ipv6 transports
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv6/af_inet6.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 4628681eca88..d5e98ee9fc79 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -564,6 +564,8 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
EXPORT_SYMBOL(inet6_ioctl);
+INDIRECT_CALLABLE_DECLARE(int udpv6_sendmsg(struct sock *, struct msghdr *,
+ size_t));
int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct sock *sk = sock->sk;
@@ -571,9 +573,12 @@ int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (unlikely(inet_send_prepare(sk)))
return -EAGAIN;
- return sk->sk_prot->sendmsg(sk, msg, size);
+ return INDIRECT_CALL_2(sk->sk_prot->sendmsg, tcp_sendmsg, udpv6_sendmsg,
+ sk, msg, size);
}
+INDIRECT_CALLABLE_DECLARE(int udpv6_recvmsg(struct sock *, struct msghdr *,
+ size_t, int, int, int *));
int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
int flags)
{
@@ -584,8 +589,9 @@ int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
if (likely(!(flags & MSG_ERRQUEUE)))
sock_rps_record_flow(sk);
- err = sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
- flags & ~MSG_DONTWAIT, &addr_len);
+ err = INDIRECT_CALL_2(sk->sk_prot->recvmsg, tcp_recvmsg, udpv6_recvmsg,
+ sk, msg, size, flags & MSG_DONTWAIT,
+ flags & ~MSG_DONTWAIT, &addr_len);
if (err >= 0)
msg->msg_namelen = addr_len;
return err;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 1/5] inet: factor out inet_send_prepare()
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
The same code is replicated verbatim in multiple places, and the next
patches will introduce an additional user for it. Factor out a
helper and use it where appropriate. No functional change intended.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/inet_common.h | 1 +
net/ipv4/af_inet.c | 21 +++++++++++++--------
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/net/inet_common.h b/include/net/inet_common.h
index 975901a95c0f..ae2ba897675c 100644
--- a/include/net/inet_common.h
+++ b/include/net/inet_common.h
@@ -25,6 +25,7 @@ int inet_dgram_connect(struct socket *sock, struct sockaddr *uaddr,
int addr_len, int flags);
int inet_accept(struct socket *sock, struct socket *newsock, int flags,
bool kern);
+int inet_send_prepare(struct sock *sk);
int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size);
ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
size_t size, int flags);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 52bdb881a506..8421e2f5bbb3 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -784,10 +784,8 @@ int inet_getname(struct socket *sock, struct sockaddr *uaddr,
}
EXPORT_SYMBOL(inet_getname);
-int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
+int inet_send_prepare(struct sock *sk)
{
- struct sock *sk = sock->sk;
-
sock_rps_record_flow(sk);
/* We may need to bind the socket. */
@@ -795,6 +793,17 @@ int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
inet_autobind(sk))
return -EAGAIN;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(inet_send_prepare);
+
+int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
+{
+ struct sock *sk = sock->sk;
+
+ if (unlikely(inet_send_prepare(sk)))
+ return -EAGAIN;
+
return sk->sk_prot->sendmsg(sk, msg, size);
}
EXPORT_SYMBOL(inet_sendmsg);
@@ -804,11 +813,7 @@ ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
{
struct sock *sk = sock->sk;
- sock_rps_record_flow(sk);
-
- /* We may need to bind the socket. */
- if (!inet_sk(sk)->inet_num && !sk->sk_prot->no_autobind &&
- inet_autobind(sk))
+ if (unlikely(inet_send_prepare(sk)))
return -EAGAIN;
if (sk->sk_prot->sendpage)
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 3/5] net: adjust socket level ICW to cope with ipv6 variant of {recv,send}msg
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
After the previous patch we have ipv{6,4} variants for {recv,send}msg,
we should use the generic _INET ICW variant to call into the proper
build-in.
This also allows dropping the now unused and rather ugly _INET4 ICW macro
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/socket.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index 963df5dbdd54..f5e0e460012b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -103,13 +103,6 @@
#include <net/busy_poll.h>
#include <linux/errqueue.h>
-/* proto_ops for ipv4 and ipv6 use the same {recv,send}msg function */
-#if IS_ENABLED(CONFIG_INET)
-#define INDIRECT_CALL_INET4(f, f1, ...) INDIRECT_CALL_1(f, f1, __VA_ARGS__)
-#else
-#define INDIRECT_CALL_INET4(f, f1, ...) f(__VA_ARGS__)
-#endif
-
#ifdef CONFIG_NET_RX_BUSY_POLL
unsigned int sysctl_net_busy_read __read_mostly;
unsigned int sysctl_net_busy_poll __read_mostly;
@@ -643,8 +636,9 @@ INDIRECT_CALLABLE_DECLARE(int inet_sendmsg(struct socket *, struct msghdr *,
size_t));
static inline int sock_sendmsg_nosec(struct socket *sock, struct msghdr *msg)
{
- int ret = INDIRECT_CALL_INET4(sock->ops->sendmsg, inet_sendmsg, sock,
- msg, msg_data_left(msg));
+ int ret = INDIRECT_CALL_INET(sock->ops->sendmsg, inet6_sendmsg,
+ inet_sendmsg, sock, msg,
+ msg_data_left(msg));
BUG_ON(ret == -EIOCBQUEUED);
return ret;
}
@@ -874,8 +868,9 @@ INDIRECT_CALLABLE_DECLARE(int inet_recvmsg(struct socket *, struct msghdr *,
static inline int sock_recvmsg_nosec(struct socket *sock, struct msghdr *msg,
int flags)
{
- return INDIRECT_CALL_INET4(sock->ops->recvmsg, inet_recvmsg, sock, msg,
- msg_data_left(msg), flags);
+ return INDIRECT_CALL_INET(sock->ops->recvmsg, inet6_recvmsg,
+ inet_recvmsg, sock, msg, msg_data_left(msg),
+ flags);
}
/**
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 2/5] ipv6: provide and use ipv6 specific version for {recv,send}msg
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
This will simplify indirect call wrapper invocation in the following
patch.
No functional change intended, any - out-of-tree - IPv6 user of
inet_{recv,send}msg can keep using the existing functions.
SCTP code still uses the existing version even for ipv6: as this series
will not add ICW for SCTP, moving to the new helper would not give
any benefit.
The only other in-kernel user of inet_{recv,send}msg is
pvcalls_conn_back_read(), but psvcalls explicitly creates only IPv4 socket,
so no need to update that code path, too.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/ipv6.h | 3 +++
net/ipv6/af_inet6.c | 35 +++++++++++++++++++++++++++++++----
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index b41f6a0fa903..aecc28dff8f8 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1089,6 +1089,9 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
int inet6_hash_connect(struct inet_timewait_death_row *death_row,
struct sock *sk);
+int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size);
+int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
+ int flags);
/*
* reassembly.c
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 7382a927d1eb..4628681eca88 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -564,6 +564,33 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
EXPORT_SYMBOL(inet6_ioctl);
+int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
+{
+ struct sock *sk = sock->sk;
+
+ if (unlikely(inet_send_prepare(sk)))
+ return -EAGAIN;
+
+ return sk->sk_prot->sendmsg(sk, msg, size);
+}
+
+int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
+ int flags)
+{
+ struct sock *sk = sock->sk;
+ int addr_len = 0;
+ int err;
+
+ if (likely(!(flags & MSG_ERRQUEUE)))
+ sock_rps_record_flow(sk);
+
+ err = sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
+ flags & ~MSG_DONTWAIT, &addr_len);
+ if (err >= 0)
+ msg->msg_namelen = addr_len;
+ return err;
+}
+
const struct proto_ops inet6_stream_ops = {
.family = PF_INET6,
.owner = THIS_MODULE,
@@ -580,8 +607,8 @@ const struct proto_ops inet6_stream_ops = {
.shutdown = inet_shutdown, /* ok */
.setsockopt = sock_common_setsockopt, /* ok */
.getsockopt = sock_common_getsockopt, /* ok */
- .sendmsg = inet_sendmsg, /* ok */
- .recvmsg = inet_recvmsg, /* ok */
+ .sendmsg = inet6_sendmsg, /* retpoline's sake */
+ .recvmsg = inet6_recvmsg, /* retpoline's sake */
#ifdef CONFIG_MMU
.mmap = tcp_mmap,
#endif
@@ -614,8 +641,8 @@ const struct proto_ops inet6_dgram_ops = {
.shutdown = inet_shutdown, /* ok */
.setsockopt = sock_common_setsockopt, /* ok */
.getsockopt = sock_common_getsockopt, /* ok */
- .sendmsg = inet_sendmsg, /* ok */
- .recvmsg = inet_recvmsg, /* ok */
+ .sendmsg = inet6_sendmsg, /* retpoline's sake */
+ .recvmsg = inet6_recvmsg, /* retpoline's sake */
.mmap = sock_no_mmap,
.sendpage = sock_no_sendpage,
.set_peek_off = sk_set_peek_off,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 0/5] net: use ICW for sk_proto->{send,recv}msg
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
This series extends ICW usage to one of the few remaining spots in fast-path
still hitting per packet retpoline overhead, namely the sk_proto->{send,recv}msg
calls.
The first 3 patches in this series refactor the existing code so that applying
the ICW macros is straight-forward: we demux inet_{recv,send}msg in ipv4 and
ipv6 variants so that each of them can easily select the appropriate TCP or UDP
direct call. While at it, a new helper is created to avoid excessive code
duplication, and the current ICWs for inet_{recv,send}msg are adjusted
accordingly.
The last 2 patches really introduce the new ICW use-case, respectively for the
ipv6 and the ipv4 code path.
This gives up to 5% performance improvement under UDP flood, and smaller but
measurable gains for TCP RR workloads.
Paolo Abeni (5):
inet: factor out inet_send_prepare()
ipv6: provide and use ipv6 specific version for {recv,send}msg
net: adjust socket level ICW to cope with ipv6 variant of
{recv,send}msg
ipv6: use indirect call wrappers for {tcp,udpv6}_{recv,send}msg()
ipv4: use indirect call wrappers for {tcp,udp}_{recv,send}msg()
include/net/inet_common.h | 1 +
include/net/ipv6.h | 3 +++
net/ipv4/af_inet.c | 33 ++++++++++++++++++++-----------
net/ipv6/af_inet6.c | 41 +++++++++++++++++++++++++++++++++++----
net/socket.c | 17 ++++++----------
5 files changed, 69 insertions(+), 26 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH v4 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Yonghong Song @ 2019-07-01 17:09 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-5-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add ability to attach to kernel and user probes and retprobes.
> Implementation depends on perf event support for kprobes/uprobes.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 165 +++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 7 ++
> tools/lib/bpf/libbpf.map | 2 +
> 3 files changed, 174 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 98c155ec3bfa..2f79e9563db9 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4019,6 +4019,171 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> return (struct bpf_link *)link;
> }
>
> +static int parse_value_from_file(const char *file, const char *fmt)
Here, the value from the file must be positive int values to avoid
confusion between valid value vs. error code.
Could you add a comment to state this fact for the current
uprobe/kprobe/tracepoint support?
> +{
> + char buf[STRERR_BUFSIZE];
> + int err, ret;
> + FILE *f;
> +
> + f = fopen(file, "r");
> + if (!f) {
> + err = -errno;
> + pr_debug("failed to open '%s': %s\n", file,
> + libbpf_strerror_r(err, buf, sizeof(buf)));
> + fclose(f);
fclose(f) is not needed. fopen has failed.
> + return err;
> + }
> + err = fscanf(f, fmt, &ret);
> + if (err != 1) {
> + err = err == EOF ? -EIO : -errno;
> + pr_debug("failed to parse '%s': %s\n", file,
> + libbpf_strerror_r(err, buf, sizeof(buf)));
> + fclose(f);
> + return err;
> + }
> + fclose(f);
> + return ret;
> +}
> +
> +static int determine_kprobe_perf_type(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/kprobe/type";
> +
> + return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int determine_uprobe_perf_type(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/uprobe/type";
> +
> + return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int determine_kprobe_retprobe_bit(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> +
> + return parse_value_from_file(file, "config:%d\n");
> +}
> +
> +static int determine_uprobe_retprobe_bit(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> +
> + return parse_value_from_file(file, "config:%d\n");
> +}
> +
> +static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name,
> + uint64_t offset, int pid)
> +{
> + struct perf_event_attr attr = {};
> + char errmsg[STRERR_BUFSIZE];
> + int type, pfd, err;
> +
> + type = uprobe ? determine_uprobe_perf_type()
> + : determine_kprobe_perf_type();
> + if (type < 0) {
> + pr_warning("failed to determine %s perf type: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> + return type;
> + }
> + if (retprobe) {
> + int bit = uprobe ? determine_uprobe_retprobe_bit()
> + : determine_kprobe_retprobe_bit();
> +
> + if (bit < 0) {
> + pr_warning("failed to determine %s retprobe bit: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(bit, errmsg,
> + sizeof(errmsg)));
> + return bit;
> + }
> + attr.config |= 1 << bit;
> + }
> + attr.size = sizeof(attr);
> + attr.type = type;
> + attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> + attr.config2 = offset; /* kprobe_addr or probe_offset */
> +
> + /* pid filter is meaningful only for uprobes */
> + pfd = syscall(__NR_perf_event_open, &attr,
> + pid < 0 ? -1 : pid /* pid */,
> + pid == -1 ? 0 : -1 /* cpu */,
> + -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> + if (pfd < 0) {
> + err = -errno;
> + pr_warning("%s perf_event_open() failed: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return err;
> + }
> + return pfd;
> +}
> +
> +struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
> + bool retprobe,
> + const char *func_name)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link *link;
> + int pfd, err;
> +
> + pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> + 0 /* offset */, -1 /* pid */);
> + if (pfd < 0) {
> + pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "kretprobe" : "kprobe", func_name,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(pfd);
> + }
> + link = bpf_program__attach_perf_event(prog, pfd);
> + if (IS_ERR(link)) {
> + close(pfd);
> + err = PTR_ERR(link);
> + pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "kretprobe" : "kprobe", func_name,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return link;
> + }
> + return link;
> +}
> +
> +struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
> + bool retprobe, pid_t pid,
> + const char *binary_path,
> + size_t func_offset)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link *link;
> + int pfd, err;
> +
> + pfd = perf_event_open_probe(true /* uprobe */, retprobe,
> + binary_path, func_offset, pid);
> + if (pfd < 0) {
> + pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "uretprobe" : "uprobe",
> + binary_path, func_offset,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(pfd);
> + }
> + link = bpf_program__attach_perf_event(prog, pfd);
> + if (IS_ERR(link)) {
> + close(pfd);
> + err = PTR_ERR(link);
> + pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "uretprobe" : "uprobe",
> + binary_path, func_offset,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return link;
> + }
> + return link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 1bf66c4a9330..bd767cc11967 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>
> LIBBPF_API struct bpf_link *
> bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> + const char *func_name);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> + pid_t pid, const char *binary_path,
> + size_t func_offset);
>
> struct bpf_insn;
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 756f5aa802e9..57a40fb60718 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
> global:
> bpf_link__destroy;
> bpf_object__load_xattr;
> + bpf_program__attach_kprobe;
> bpf_program__attach_perf_event;
> + bpf_program__attach_uprobe;
> btf_dump__dump_type;
> btf_dump__free;
> btf_dump__new;
>
^ permalink raw reply
* Re: [PATCH net-next 0/7] net/rds: RDMA fixes
From: santosh.shilimkar @ 2019-07-01 17:06 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <f1f5ca90-a98a-4c7a-c918-ef26f02f6ee7@oracle.com>
On 7/1/19 9:39 AM, Gerd Rausch wrote:
> A number of net/rds fixes necessary to make "rds_rdma.ko"
> pass some basic Oracle internal tests.
>
> Gerd Rausch (7):
> net/rds: Give fr_state a chance to transition to FRMR_IS_FREE
> net/rds: Get rid of "wait_clean_list_grace" and add locking
> net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after
> posting IB_WR_LOCAL_INV
> net/rds: Fix NULL/ERR_PTR inconsistency
> net/rds: Set fr_state only to FRMR_IS_FREE if IB_WR_LOCAL_INV had been
> successful
> net/rds: Keep track of and wait for FRWR segments in use upon shutdown
> net/rds: Initialize ic->i_fastreg_wrs upon allocation
>
Will apply these on top of earlier few fixes after going through them.
Thanks for posting them out.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH v4 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event
From: Yonghong Song @ 2019-07-01 17:03 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-4-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> bpf_program__attach_perf_event allows to attach BPF program to existing
> perf event hook, providing most generic and most low-level way to attach BPF
> programs. It returns struct bpf_link, which should be passed to
> bpf_link__destroy to detach and free resources, associated with a link.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 61 ++++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 3 ++
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 65 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 455795e6f8af..98c155ec3bfa 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -32,6 +32,7 @@
> #include <linux/limits.h>
> #include <linux/perf_event.h>
> #include <linux/ring_buffer.h>
> +#include <sys/ioctl.h>
> #include <sys/stat.h>
> #include <sys/types.h>
> #include <sys/vfs.h>
> @@ -3958,6 +3959,66 @@ int bpf_link__destroy(struct bpf_link *link)
> return err;
> }
>
> +struct bpf_link_fd {
> + struct bpf_link link; /* has to be at the top of struct */
> + int fd; /* hook FD */
> +};
> +
> +static int bpf_link__destroy_perf_event(struct bpf_link *link)
> +{
> + struct bpf_link_fd *l = (void *)link;
> + int err;
> +
> + if (l->fd < 0)
> + return 0;
> +
> + err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
> + if (err)
> + err = -errno;
> +
> + close(l->fd);
> + return err;
> +}
> +
> +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> + int pfd)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link_fd *link;
> + int prog_fd, err;
> +
> + prog_fd = bpf_program__fd(prog);
> + if (prog_fd < 0) {
> + pr_warning("program '%s': can't attach before loaded\n",
> + bpf_program__title(prog, false));
> + return ERR_PTR(-EINVAL);
> + }
should we check validity of pfd here?
If pfd < 0, we just return ERR_PTR(-EINVAL)?
This way, in bpf_link__destroy_perf_event(), we do not need to check
l->fd < 0 since it will be always nonnegative.
> +
> + link = malloc(sizeof(*link));
> + if (!link)
> + return ERR_PTR(-ENOMEM);
> + link->link.destroy = &bpf_link__destroy_perf_event;
> + link->fd = pfd;
> +
> + if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) {
> + err = -errno;
> + free(link);
> + pr_warning("program '%s': failed to attach to pfd %d: %s\n",
> + bpf_program__title(prog, false), pfd,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return ERR_PTR(err);
> + }
> + if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
> + err = -errno;
> + free(link);
> + pr_warning("program '%s': failed to enable pfd %d: %s\n",
> + bpf_program__title(prog, false), pfd,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return ERR_PTR(err);
> + }
> + return (struct bpf_link *)link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5082a5ebb0c2..1bf66c4a9330 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -169,6 +169,9 @@ struct bpf_link;
>
> LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +
> struct bpf_insn;
>
> /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 3cde850fc8da..756f5aa802e9 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
> global:
> bpf_link__destroy;
> bpf_object__load_xattr;
> + bpf_program__attach_perf_event;
> btf_dump__dump_type;
> btf_dump__free;
> btf_dump__new;
>
^ permalink raw reply
* Re: [PATCH v2 0/3] vsock/virtio: several fixes in the .probe() and .remove()
From: Stefano Garzarella @ 2019-07-01 17:03 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190701151113.GE11900@stefanha-x1.localdomain>
On Mon, Jul 01, 2019 at 04:11:13PM +0100, Stefan Hajnoczi wrote:
> On Fri, Jun 28, 2019 at 02:36:56PM +0200, Stefano Garzarella wrote:
> > During the review of "[PATCH] vsock/virtio: Initialize core virtio vsock
> > before registering the driver", Stefan pointed out some possible issues
> > in the .probe() and .remove() callbacks of the virtio-vsock driver.
> >
> > This series tries to solve these issues:
> > - Patch 1 adds RCU critical sections to avoid use-after-free of
> > 'the_virtio_vsock' pointer.
> > - Patch 2 stops workers before to call vdev->config->reset(vdev) to
> > be sure that no one is accessing the device.
> > - Patch 3 moves the works flush at the end of the .remove() to avoid
> > use-after-free of 'vsock' object.
> >
> > v2:
> > - Patch 1: use RCU to protect 'the_virtio_vsock' pointer
> > - Patch 2: no changes
> > - Patch 3: flush works only at the end of .remove()
> > - Removed patch 4 because virtqueue_detach_unused_buf() returns all the buffers
> > allocated.
> >
> > v1: https://patchwork.kernel.org/cover/10964733/
>
> This looks good to me.
Thanks for the review!
>
> Did you run any stress tests? For example an SMP guest constantly
> connecting and sending packets together with a script that
> hotplug/unplugs vhost-vsock-pci from the host side.
Yes, I started an SMP guest (-smp 4 -monitor tcp:127.0.0.1:1234,server,nowait)
and I run these scripts to stress the .probe()/.remove() path:
- guest
while true; do
cat /dev/urandom | nc-vsock -l 4321 > /dev/null &
cat /dev/urandom | nc-vsock -l 5321 > /dev/null &
cat /dev/urandom | nc-vsock -l 6321 > /dev/null &
cat /dev/urandom | nc-vsock -l 7321 > /dev/null &
wait
done
- host
while true; do
cat /dev/urandom | nc-vsock 3 4321 > /dev/null &
cat /dev/urandom | nc-vsock 3 5321 > /dev/null &
cat /dev/urandom | nc-vsock 3 6321 > /dev/null &
cat /dev/urandom | nc-vsock 3 7321 > /dev/null &
sleep 2
echo "device_del v1" | nc 127.0.0.1 1234
sleep 1
echo "device_add vhost-vsock-pci,id=v1,guest-cid=3" | nc 127.0.0.1 1234
sleep 1
done
Do you think is enough or is better to have a test more accurate?
Thanks,
Stefano
^ permalink raw reply
* Re: Use-after-free in br_multicast_rcv
From: Nikolay Aleksandrov @ 2019-07-01 17:03 UTC (permalink / raw)
To: Martin Weinelt, bridge, Roopa Prabhu; +Cc: netdev
In-Reply-To: <21ab085f-0f7f-88bc-b661-af74dd9eeea2@linuxlounge.net>
Hi Martin,
On 01/07/2019 19:53, Martin Weinelt wrote:
> Hi Nik,
>
> more info below.
>
> On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
>> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>>> Hello,
>>>
>>> we've recently been experiencing memory leaks on our Linux-based
>>> routers,
>>> at least as far back as v4.19.16.
>>>
>>> After rebuilding with KASAN it found a use-after-free in
>>> br_multicast_rcv which I could reproduce on v5.2.0-rc6.
>>>
>>> Please find the KASAN report below, I'm anot sure what else to provide
>>> so
>>> feel free to ask.
>>>
>>> Best,
>>> Martin
>>>
>>>
>>
>> Hi Martin,
>> I'll look into this, are there any specific steps to reproduce it?
>>
>> Thanks,
>> Nik
>>>
> Each server is a KVM Guest and has 18 bridges with the same master/slave
> relationships:
>
> bridge -> batman-adv -> {l2 tunnel, virtio device}
>
> Linus Lüssing from the batman-adv asked me to apply this patch to help
> debugging.
>
> v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN
> report, not sure if the additional information at the end is a result of
> the added patch though.
>
> Best,
> Martin
>
I see a couple of issues that can cause out-of-bounds accesses in br_multicast.c
more specifically there're pskb_may_pull calls and accesses to stale skb pointers.
I've had these on my "to fix" list for some time now, will prepare, test the fixes and
send them for review. In a few minutes I'll send a test patch for you.
That being said, I thought you said you've been experiencing memory leaks, but below
reports are for out-of-bounds accesses, could you please clarify if you were
speaking about these or is there another issue as well ?
If you're experiencing memory leaks, are you sure they're related to the bridge ?
You could try kmemleak for those.
Thank you,
Nik
> From 47a04e977311a0c45f26905588f563b55239da7f Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Linus=20L=C3=BCssing?= <linus.luessing@c0d3.blue>
> Date: Sat, 29 Jun 2019 20:24:23 +0200
> Subject: [PATCH] bridge: DEBUG: ipv6_addr_is_ll_all_nodes() wrappers for impr.
> call traces
>
> ---
> net/bridge/br_multicast.c | 70 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 63 insertions(+), 7 deletions(-)
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index de22c8fbbb15..224a43318955 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -57,6 +57,42 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
> struct net_bridge_port *port,
> const struct in6_addr *group,
> __u16 vid, const unsigned char *src);
> +
> +static noinline void br_ip6_multicast_leave_group_mld2report(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + br_ip6_multicast_leave_group(br, port, group, vid, src);
> +}
> +
> +static noinline void br_ip6_multicast_leave_group_ipv6rcv(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + br_ip6_multicast_leave_group(br, port, group, vid, src);
> +}
> +
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_addgroup(const struct in6_addr *addr)
> +{
> + return ipv6_addr_is_ll_all_nodes(addr);
> +}
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_leavegroup(const struct in6_addr *addr)
> +{
> + return ipv6_addr_is_ll_all_nodes(addr);
> +}
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_mcastrcv(const struct in6_addr *addr)
> +{
> + return ipv6_addr_is_ll_all_nodes(addr);
> +}
> #endif
>
> static struct net_bridge_mdb_entry *br_mdb_ip_get_rcu(struct net_bridge *br,
> @@ -595,7 +631,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
> {
> struct br_ip br_group;
>
> - if (ipv6_addr_is_ll_all_nodes(group))
> + if (ipv6_addr_is_ll_all_nodes_addgroup(group))
> return 0;
>
> memset(&br_group, 0, sizeof(br_group));
> @@ -605,6 +641,26 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
>
> return br_multicast_add_group(br, port, &br_group, src);
> }
> +
> +static noinline int br_ip6_multicast_add_group_mld2report(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + return br_ip6_multicast_add_group(br, port, group, vid, src);
> +}
> +
> +static noinline int br_ip6_multicast_add_group_ipv6rcv(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + return br_ip6_multicast_add_group(br, port, group, vid, src);
> +}
> #endif
>
> static void br_multicast_router_expired(struct timer_list *t)
> @@ -1022,10 +1078,10 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
> if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
> grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
> ntohs(*nsrcs) == 0) {
> - br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
> + br_ip6_multicast_leave_group_mld2report(br, port, &grec->grec_mca,
> vid, src);
> } else {
> - err = br_ip6_multicast_add_group(br, port,
> + err = br_ip6_multicast_add_group_mld2report(br, port,
> &grec->grec_mca, vid,
> src);
> if (err)
> @@ -1494,7 +1550,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
> struct br_ip br_group;
> struct bridge_mcast_own_query *own_query;
>
> - if (ipv6_addr_is_ll_all_nodes(group))
> + if (ipv6_addr_is_ll_all_nodes_leavegroup(group))
> return;
>
> own_query = port ? &port->ip6_own_query : &br->ip6_own_query;
> @@ -1658,7 +1714,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> err = ipv6_mc_check_mld(skb);
>
> if (err == -ENOMSG) {
> - if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr))
> + if (!ipv6_addr_is_ll_all_nodes_mcastrcv(&ipv6_hdr(skb)->daddr))
> BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
>
> if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) {
> @@ -1683,7 +1739,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> case ICMPV6_MGM_REPORT:
> src = eth_hdr(skb)->h_source;
> BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
> - err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid,
> + err = br_ip6_multicast_add_group_ipv6rcv(br, port, &mld->mld_mca, vid,
> src);
> break;
> case ICMPV6_MLD2_REPORT:
> @@ -1694,7 +1750,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> break;
> case ICMPV6_MGM_REDUCTION:
> src = eth_hdr(skb)->h_source;
> - br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid, src);
> + br_ip6_multicast_leave_group_ipv6rcv(br, port, &mld->mld_mca, vid, src);
> break;
> }
>
>
^ permalink raw reply
* [PATCH net v2] ipv4: don't set IPv6 only flags to IPv4 addresses
From: Matteo Croce @ 2019-07-01 17:01 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI
Avoid the situation where an IPV6 only flag is applied to an IPv4 address:
# ip addr add 192.0.2.1/24 dev dummy0 nodad home mngtmpaddr noprefixroute
# ip -4 addr show dev dummy0
2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
inet 192.0.2.1/24 scope global noprefixroute dummy0
valid_lft forever preferred_lft forever
Or worse, by sending a malicious netlink command:
# ip -4 addr show dev dummy0
2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
inet 192.0.2.1/24 scope global nodad optimistic dadfailed home tentative mngtmpaddr noprefixroute stable-privacy dummy0
valid_lft forever preferred_lft forever
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
net/ipv4/devinet.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index c6bd0f7a020a..c5ebfa199794 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -62,6 +62,11 @@
#include <net/net_namespace.h>
#include <net/addrconf.h>
+#define IPV6ONLY_FLAGS \
+ (IFA_F_NODAD | IFA_F_OPTIMISTIC | IFA_F_DADFAILED | \
+ IFA_F_HOMEADDRESS | IFA_F_TENTATIVE | \
+ IFA_F_MANAGETEMPADDR | IFA_F_STABLE_PRIVACY)
+
static struct ipv4_devconf ipv4_devconf = {
.data = {
[IPV4_DEVCONF_ACCEPT_REDIRECTS - 1] = 1,
@@ -468,6 +473,9 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
ifa->ifa_flags &= ~IFA_F_SECONDARY;
last_primary = &in_dev->ifa_list;
+ /* Don't set IPv6 only flags to IPv4 addresses */
+ ifa->ifa_flags &= ~IPV6ONLY_FLAGS;
+
for (ifap = &in_dev->ifa_list; (ifa1 = *ifap) != NULL;
ifap = &ifa1->ifa_next) {
if (!(ifa1->ifa_flags & IFA_F_SECONDARY) &&
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v4 bpf-next 2/9] libbpf: introduce concept of bpf_link
From: Yonghong Song @ 2019-07-01 17:01 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-3-andriin@fb.com>
On 6/28/19 8:48 PM, Andrii Nakryiko wrote:
> bpf_link is and abstraction of an association of a BPF program and one
"is and" => "is an".
> of many possible BPF attachment points (hooks). This allows to have
> uniform interface for detaching BPF programs regardless of the nature of
> link and how it was created. Details of creation and setting up of
> a specific bpf_link is handled by corresponding attachment methods
> (bpf_program__attach_xxx) added in subsequent commits. Once successfully
> created, bpf_link has to be eventually destroyed with
> bpf_link__destroy(), at which point BPF program is disassociated from
> a hook and all the relevant resources are freed.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 17 +++++++++++++++++
> tools/lib/bpf/libbpf.h | 4 ++++
> tools/lib/bpf/libbpf.map | 3 ++-
> 3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6e6ebef11ba3..455795e6f8af 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3941,6 +3941,23 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> return 0;
> }
>
> +struct bpf_link {
> + int (*destroy)(struct bpf_link *link);
> +};
> +
> +int bpf_link__destroy(struct bpf_link *link)
> +{
> + int err;
> +
> + if (!link)
> + return 0;
> +
> + err = link->destroy(link);
> + free(link);
> +
> + return err;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index d639f47e3110..5082a5ebb0c2 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -165,6 +165,10 @@ LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
> LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
> LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
>
> +struct bpf_link;
> +
> +LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> +
> struct bpf_insn;
>
> /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 2c6d835620d2..3cde850fc8da 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -167,10 +167,11 @@ LIBBPF_0.0.3 {
>
> LIBBPF_0.0.4 {
> global:
> + bpf_link__destroy;
> + bpf_object__load_xattr;
> btf_dump__dump_type;
> btf_dump__free;
> btf_dump__new;
> btf__parse_elf;
> - bpf_object__load_xattr;
> libbpf_num_possible_cpus;
> } LIBBPF_0.0.3;
>
^ permalink raw reply
* [PATCH net-next] tipc: remove ub->ubsock checks
From: Xin Long @ 2019-07-01 16:57 UTC (permalink / raw)
To: network dev; +Cc: Jon Maloy, Ying Xue, tipc-discussion, davem
Both tipc_udp_enable and tipc_udp_disable are called under rtnl_lock,
ub->ubsock could never be NULL in tipc_udp_disable and cleanup_bearer,
so remove the check.
Also remove the one in tipc_udp_enable by adding "free" label.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/udp_media.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index 62b85db..287df687 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -759,7 +759,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
err = dst_cache_init(&ub->rcast.dst_cache, GFP_ATOMIC);
if (err)
- goto err;
+ goto free;
/**
* The bcast media address port is used for all peers and the ip
@@ -771,13 +771,14 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
else
err = tipc_udp_rcast_add(b, &remote);
if (err)
- goto err;
+ goto free;
return 0;
-err:
+
+free:
dst_cache_destroy(&ub->rcast.dst_cache);
- if (ub->ubsock)
- udp_tunnel_sock_release(ub->ubsock);
+ udp_tunnel_sock_release(ub->ubsock);
+err:
kfree(ub);
return err;
}
@@ -795,8 +796,7 @@ static void cleanup_bearer(struct work_struct *work)
}
dst_cache_destroy(&ub->rcast.dst_cache);
- if (ub->ubsock)
- udp_tunnel_sock_release(ub->ubsock);
+ udp_tunnel_sock_release(ub->ubsock);
synchronize_net();
kfree(ub);
}
@@ -811,8 +811,7 @@ static void tipc_udp_disable(struct tipc_bearer *b)
pr_err("UDP bearer instance not found\n");
return;
}
- if (ub->ubsock)
- sock_set_flag(ub->ubsock->sk, SOCK_DEAD);
+ sock_set_flag(ub->ubsock->sk, SOCK_DEAD);
RCU_INIT_POINTER(ub->bearer, NULL);
/* sock_release need to be done outside of rtnl lock */
--
2.1.0
^ permalink raw reply related
* Re: [PATCH bpf-next v2 3/3] selftests/bpf: add verifier tests for wide stores
From: Andrii Nakryiko @ 2019-07-01 16:55 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Networking, bpf, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Yonghong Song
In-Reply-To: <20190701163103.237550-4-sdf@google.com>
On Mon, Jul 1, 2019 at 9:54 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> Make sure that wide stores are allowed at proper (aligned) addresses.
> Note that user_ip6 is naturally aligned on 8-byte boundary, so
> correct addresses are user_ip6[0] and user_ip6[2]. msg_src_ip6 is,
> however, aligned on a 4-byte bondary, so only msg_src_ip6[1]
> can be wide-stored.
>
> Cc: Andrii Nakryiko <andriin@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
Acked-by: Andrii Nakryiko <andriin@fb.com>
> tools/testing/selftests/bpf/test_verifier.c | 17 +++++++--
> .../selftests/bpf/verifier/wide_store.c | 36 +++++++++++++++++++
> 2 files changed, 50 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/verifier/wide_store.c
>
<snip>
^ permalink raw reply
* [PATCH net-next] tipc: use rcu dereference functions properly
From: Xin Long @ 2019-07-01 16:54 UTC (permalink / raw)
To: network dev; +Cc: Jon Maloy, Ying Xue, tipc-discussion, davem
For these places are protected by rcu_read_lock, we change from
rcu_dereference_rtnl to rcu_dereference, as there is no need to
check if rtnl lock is held.
For these places are protected by rtnl_lock, we change from
rcu_dereference_rtnl to rtnl_dereference/rcu_dereference_protected,
as no extra memory barriers are needed under rtnl_lock() which also
protects tn->bearer_list[] and dev->tipc_ptr/b->media_ptr updating.
rcu_dereference_rtnl will be only used in the places where it could
be under rcu_read_lock or rtnl_lock.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/bearer.c | 14 +++++++-------
net/tipc/udp_media.c | 8 ++++----
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 2bed658..a809c0e 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -62,7 +62,7 @@ static struct tipc_bearer *bearer_get(struct net *net, int bearer_id)
{
struct tipc_net *tn = tipc_net(net);
- return rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+ return rcu_dereference(tn->bearer_list[bearer_id]);
}
static void bearer_disable(struct net *net, struct tipc_bearer *b);
@@ -210,7 +210,7 @@ void tipc_bearer_add_dest(struct net *net, u32 bearer_id, u32 dest)
struct tipc_bearer *b;
rcu_read_lock();
- b = rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+ b = rcu_dereference(tn->bearer_list[bearer_id]);
if (b)
tipc_disc_add_dest(b->disc);
rcu_read_unlock();
@@ -222,7 +222,7 @@ void tipc_bearer_remove_dest(struct net *net, u32 bearer_id, u32 dest)
struct tipc_bearer *b;
rcu_read_lock();
- b = rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+ b = rcu_dereference(tn->bearer_list[bearer_id]);
if (b)
tipc_disc_remove_dest(b->disc);
rcu_read_unlock();
@@ -444,7 +444,7 @@ int tipc_l2_send_msg(struct net *net, struct sk_buff *skb,
struct net_device *dev;
int delta;
- dev = (struct net_device *)rcu_dereference_rtnl(b->media_ptr);
+ dev = (struct net_device *)rcu_dereference(b->media_ptr);
if (!dev)
return 0;
@@ -481,7 +481,7 @@ int tipc_bearer_mtu(struct net *net, u32 bearer_id)
struct tipc_bearer *b;
rcu_read_lock();
- b = rcu_dereference_rtnl(tipc_net(net)->bearer_list[bearer_id]);
+ b = rcu_dereference(tipc_net(net)->bearer_list[bearer_id]);
if (b)
mtu = b->mtu;
rcu_read_unlock();
@@ -574,8 +574,8 @@ static int tipc_l2_rcv_msg(struct sk_buff *skb, struct net_device *dev,
struct tipc_bearer *b;
rcu_read_lock();
- b = rcu_dereference_rtnl(dev->tipc_ptr) ?:
- rcu_dereference_rtnl(orig_dev->tipc_ptr);
+ b = rcu_dereference(dev->tipc_ptr) ?:
+ rcu_dereference(orig_dev->tipc_ptr);
if (likely(b && test_bit(0, &b->up) &&
(skb->pkt_type <= PACKET_MULTICAST))) {
skb_mark_not_on_list(skb);
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index b8962df..62b85db 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -231,7 +231,7 @@ static int tipc_udp_send_msg(struct net *net, struct sk_buff *skb,
}
skb_set_inner_protocol(skb, htons(ETH_P_TIPC));
- ub = rcu_dereference_rtnl(b->media_ptr);
+ ub = rcu_dereference(b->media_ptr);
if (!ub) {
err = -ENODEV;
goto out;
@@ -490,7 +490,7 @@ int tipc_udp_nl_dump_remoteip(struct sk_buff *skb, struct netlink_callback *cb)
}
}
- ub = rcu_dereference_rtnl(b->media_ptr);
+ ub = rtnl_dereference(b->media_ptr);
if (!ub) {
rtnl_unlock();
return -EINVAL;
@@ -532,7 +532,7 @@ int tipc_udp_nl_add_bearer_data(struct tipc_nl_msg *msg, struct tipc_bearer *b)
struct udp_bearer *ub;
struct nlattr *nest;
- ub = rcu_dereference_rtnl(b->media_ptr);
+ ub = rtnl_dereference(b->media_ptr);
if (!ub)
return -ENODEV;
@@ -806,7 +806,7 @@ static void tipc_udp_disable(struct tipc_bearer *b)
{
struct udp_bearer *ub;
- ub = rcu_dereference_rtnl(b->media_ptr);
+ ub = rtnl_dereference(b->media_ptr);
if (!ub) {
pr_err("UDP bearer instance not found\n");
return;
--
2.1.0
^ permalink raw reply related
* Re: [PATCH bpf-next v2 1/3] bpf: allow wide (u64) aligned stores for some fields of bpf_sock_addr
From: Andrii Nakryiko @ 2019-07-01 16:54 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Networking, bpf, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Yonghong Song,
kernel test robot
In-Reply-To: <20190701163103.237550-2-sdf@google.com>
On Mon, Jul 1, 2019 at 9:51 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> Since commit cd17d7770578 ("bpf/tools: sync bpf.h") clang decided
> that it can do a single u64 store into user_ip6[2] instead of two
> separate u32 ones:
>
> # 17: (18) r2 = 0x100000000000000
> # ; ctx->user_ip6[2] = bpf_htonl(DST_REWRITE_IP6_2);
> # 19: (7b) *(u64 *)(r1 +16) = r2
> # invalid bpf_context access off=16 size=8
>
> From the compiler point of view it does look like a correct thing
> to do, so let's support it on the kernel side.
>
> Credit to Andrii Nakryiko for a proper implementation of
> bpf_ctx_wide_store_ok.
>
> Cc: Andrii Nakryiko <andriin@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Fixes: cd17d7770578 ("bpf/tools: sync bpf.h")
> Reported-by: kernel test robot <rong.a.chen@intel.com>
> Acked-by: Yonghong Song <yhs@fb.com>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
> include/linux/filter.h | 6 ++++++
> include/uapi/linux/bpf.h | 4 ++--
> net/core/filter.c | 22 ++++++++++++++--------
> 3 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 340f7d648974..3901007e36f1 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -746,6 +746,12 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
> return size <= size_default && (size & (size - 1)) == 0;
> }
>
> +#define bpf_ctx_wide_store_ok(off, size, type, field) \
> + (size == sizeof(__u64) && \
> + off >= offsetof(type, field) && \
> + off + sizeof(__u64) <= offsetofend(type, field) && \
> + off % sizeof(__u64) == 0)
> +
> #define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
>
> static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index a396b516a2b2..586867fe6102 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3237,7 +3237,7 @@ struct bpf_sock_addr {
> __u32 user_ip4; /* Allows 1,2,4-byte read and 4-byte write.
> * Stored in network byte order.
> */
> - __u32 user_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
> + __u32 user_ip6[4]; /* Allows 1,2,4-byte read an 4,8-byte write.
typo: an -> and
> * Stored in network byte order.
> */
> __u32 user_port; /* Allows 4-byte read and write.
> @@ -3249,7 +3249,7 @@ struct bpf_sock_addr {
> __u32 msg_src_ip4; /* Allows 1,2,4-byte read an 4-byte write.
same
> * Stored in network byte order.
> */
> - __u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
> + __u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read an 4,8-byte write.
the power of copy/paste! :)
> * Stored in network byte order.
> */
> __bpf_md_ptr(struct bpf_sock *, sk);
> diff --git a/net/core/filter.c b/net/core/filter.c
> index dc8534be12fc..5d33f2146dab 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -6849,6 +6849,16 @@ static bool sock_addr_is_valid_access(int off, int size,
> if (!bpf_ctx_narrow_access_ok(off, size, size_default))
> return false;
> } else {
> + if (bpf_ctx_wide_store_ok(off, size,
> + struct bpf_sock_addr,
> + user_ip6))
> + return true;
> +
> + if (bpf_ctx_wide_store_ok(off, size,
> + struct bpf_sock_addr,
> + msg_src_ip6))
> + return true;
> +
> if (size != size_default)
> return false;
> }
> @@ -7689,9 +7699,6 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> /* SOCK_ADDR_STORE_NESTED_FIELD_OFF() has semantic similar to
> * SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF() but for store operation.
> *
> - * It doesn't support SIZE argument though since narrow stores are not
> - * supported for now.
> - *
> * In addition it uses Temporary Field TF (member of struct S) as the 3rd
> * "register" since two registers available in convert_ctx_access are not
> * enough: we can't override neither SRC, since it contains value to store, nor
> @@ -7699,7 +7706,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> * instructions. But we need a temporary place to save pointer to nested
> * structure whose field we want to store to.
> */
> -#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, TF) \
> +#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, OFF, TF) \
> do { \
> int tmp_reg = BPF_REG_9; \
> if (si->src_reg == tmp_reg || si->dst_reg == tmp_reg) \
> @@ -7710,8 +7717,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> offsetof(S, TF)); \
> *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(S, F), tmp_reg, \
> si->dst_reg, offsetof(S, F)); \
> - *insn++ = BPF_STX_MEM( \
> - BPF_FIELD_SIZEOF(NS, NF), tmp_reg, si->src_reg, \
> + *insn++ = BPF_STX_MEM(SIZE, tmp_reg, si->src_reg, \
> bpf_target_off(NS, NF, FIELD_SIZEOF(NS, NF), \
> target_size) \
> + OFF); \
> @@ -7723,8 +7729,8 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> TF) \
> do { \
> if (type == BPF_WRITE) { \
> - SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, \
> - TF); \
> + SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, \
> + OFF, TF); \
> } else { \
> SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF( \
> S, NS, F, NF, SIZE, OFF); \
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply
* Re: Use-after-free in br_multicast_rcv
From: Martin Weinelt @ 2019-07-01 16:53 UTC (permalink / raw)
To: nikolay, bridge, Roopa Prabhu; +Cc: netdev
In-Reply-To: <E0170D52-C181-4F0F-B5F8-F1801C2A8F5A@cumulusnetworks.com>
Hi Nik,
more info below.
On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>> Hello,
>>
>> we've recently been experiencing memory leaks on our Linux-based
>> routers,
>> at least as far back as v4.19.16.
>>
>> After rebuilding with KASAN it found a use-after-free in
>> br_multicast_rcv which I could reproduce on v5.2.0-rc6.
>>
>> Please find the KASAN report below, I'm anot sure what else to provide
>> so
>> feel free to ask.
>>
>> Best,
>> Martin
>>
>>
>
> Hi Martin,
> I'll look into this, are there any specific steps to reproduce it?
>
> Thanks,
> Nik
>
Each server is a KVM Guest and has 18 bridges with the same master/slave
relationships:
bridge -> batman-adv -> {l2 tunnel, virtio device}
Linus Lüssing from the batman-adv asked me to apply this patch to help
debugging.
v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN
report, not sure if the additional information at the end is a result of
the added patch though.
Best,
Martin
From 47a04e977311a0c45f26905588f563b55239da7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Linus=20L=C3=BCssing?= <linus.luessing@c0d3.blue>
Date: Sat, 29 Jun 2019 20:24:23 +0200
Subject: [PATCH] bridge: DEBUG: ipv6_addr_is_ll_all_nodes() wrappers for impr.
call traces
---
net/bridge/br_multicast.c | 70 +++++++++++++++++++++++++++++++++++----
1 file changed, 63 insertions(+), 7 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index de22c8fbbb15..224a43318955 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -57,6 +57,42 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
const struct in6_addr *group,
__u16 vid, const unsigned char *src);
+
+static noinline void br_ip6_multicast_leave_group_mld2report(
+ struct net_bridge *br,
+ struct net_bridge_port *port,
+ const struct in6_addr *group,
+ __u16 vid,
+ const unsigned char *src)
+{
+ br_ip6_multicast_leave_group(br, port, group, vid, src);
+}
+
+static noinline void br_ip6_multicast_leave_group_ipv6rcv(
+ struct net_bridge *br,
+ struct net_bridge_port *port,
+ const struct in6_addr *group,
+ __u16 vid,
+ const unsigned char *src)
+{
+ br_ip6_multicast_leave_group(br, port, group, vid, src);
+}
+
+
+static noinline bool ipv6_addr_is_ll_all_nodes_addgroup(const struct in6_addr *addr)
+{
+ return ipv6_addr_is_ll_all_nodes(addr);
+}
+
+static noinline bool ipv6_addr_is_ll_all_nodes_leavegroup(const struct in6_addr *addr)
+{
+ return ipv6_addr_is_ll_all_nodes(addr);
+}
+
+static noinline bool ipv6_addr_is_ll_all_nodes_mcastrcv(const struct in6_addr *addr)
+{
+ return ipv6_addr_is_ll_all_nodes(addr);
+}
#endif
static struct net_bridge_mdb_entry *br_mdb_ip_get_rcu(struct net_bridge *br,
@@ -595,7 +631,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
{
struct br_ip br_group;
- if (ipv6_addr_is_ll_all_nodes(group))
+ if (ipv6_addr_is_ll_all_nodes_addgroup(group))
return 0;
memset(&br_group, 0, sizeof(br_group));
@@ -605,6 +641,26 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
return br_multicast_add_group(br, port, &br_group, src);
}
+
+static noinline int br_ip6_multicast_add_group_mld2report(
+ struct net_bridge *br,
+ struct net_bridge_port *port,
+ const struct in6_addr *group,
+ __u16 vid,
+ const unsigned char *src)
+{
+ return br_ip6_multicast_add_group(br, port, group, vid, src);
+}
+
+static noinline int br_ip6_multicast_add_group_ipv6rcv(
+ struct net_bridge *br,
+ struct net_bridge_port *port,
+ const struct in6_addr *group,
+ __u16 vid,
+ const unsigned char *src)
+{
+ return br_ip6_multicast_add_group(br, port, group, vid, src);
+}
#endif
static void br_multicast_router_expired(struct timer_list *t)
@@ -1022,10 +1078,10 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
ntohs(*nsrcs) == 0) {
- br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
+ br_ip6_multicast_leave_group_mld2report(br, port, &grec->grec_mca,
vid, src);
} else {
- err = br_ip6_multicast_add_group(br, port,
+ err = br_ip6_multicast_add_group_mld2report(br, port,
&grec->grec_mca, vid,
src);
if (err)
@@ -1494,7 +1550,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
struct br_ip br_group;
struct bridge_mcast_own_query *own_query;
- if (ipv6_addr_is_ll_all_nodes(group))
+ if (ipv6_addr_is_ll_all_nodes_leavegroup(group))
return;
own_query = port ? &port->ip6_own_query : &br->ip6_own_query;
@@ -1658,7 +1714,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
err = ipv6_mc_check_mld(skb);
if (err == -ENOMSG) {
- if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr))
+ if (!ipv6_addr_is_ll_all_nodes_mcastrcv(&ipv6_hdr(skb)->daddr))
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) {
@@ -1683,7 +1739,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
case ICMPV6_MGM_REPORT:
src = eth_hdr(skb)->h_source;
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
- err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid,
+ err = br_ip6_multicast_add_group_ipv6rcv(br, port, &mld->mld_mca, vid,
src);
break;
case ICMPV6_MLD2_REPORT:
@@ -1694,7 +1750,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
break;
case ICMPV6_MGM_REDUCTION:
src = eth_hdr(skb)->h_source;
- br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid, src);
+ br_ip6_multicast_leave_group_ipv6rcv(br, port, &mld->mld_mca, vid, src);
break;
}
--
2.20.1
[ 409.353973] ==================================================================
[ 409.356970] BUG: KASAN: out-of-bounds in br_multicast_rcv+0x36e9/0x4080 [bridge]
[ 409.359360] Read of size 2 at addr ffff888046e980b8 by task swapper/2/0
[ 409.361668]
[ 409.362215] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G OE 5.2.0-rc6+ #1
[ 409.364503] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[ 409.367006] Call Trace:
[ 409.367699] <IRQ>
[ 409.368283] dump_stack+0x71/0xab
[ 409.369175] print_address_description+0x6a/0x280
[ 409.370350] ? br_multicast_rcv+0x36e9/0x4080 [bridge]
[ 409.371752] __kasan_report+0x152/0x1aa
[ 409.372703] ? br_multicast_rcv+0x36e9/0x4080 [bridge]
[ 409.373950] ? br_multicast_rcv+0x36e9/0x4080 [bridge]
[ 409.375308] kasan_report+0xe/0x20
[ 409.376149] br_multicast_rcv+0x36e9/0x4080 [bridge]
[ 409.377460] ? br_multicast_disable_port+0x150/0x150 [bridge]
[ 409.379079] ? kvm_clock_get_cycles+0xd/0x10
[ 409.380291] ? __kasan_kmalloc.constprop.6+0xa6/0xf0
[ 409.381693] ? netif_receive_skb_internal+0x84/0x1a0
[ 409.383075] ? __netif_receive_skb+0x1b0/0x1b0
[ 409.384355] ? br_fdb_update+0x10e/0x6e0 [bridge]
[ 409.385700] ? br_handle_frame_finish+0x3c6/0x11d0 [bridge]
[ 409.387283] br_handle_frame_finish+0x3c6/0x11d0 [bridge]
[ 409.388807] ? br_pass_frame_up+0x3a0/0x3a0 [bridge]
[ 409.390200] ? virtnet_probe+0x1c80/0x1c80 [virtio_net]
[ 409.391685] br_handle_frame+0x731/0xd90 [bridge]
[ 409.393017] ? br_handle_frame_finish+0x11d0/0x11d0 [bridge]
[ 409.394591] ? __update_load_avg_se+0x592/0xa70
[ 409.395862] ? __update_load_avg_se+0x592/0xa70
[ 409.397065] __netif_receive_skb_core+0xced/0x2d70
[ 409.398208] ? napi_complete_done+0x10/0x360
[ 409.399249] ? virtqueue_get_buf_ctx+0x271/0x1130 [virtio_ring]
[ 409.400632] ? do_xdp_generic+0x20/0x20
[ 409.401565] ? virtqueue_napi_complete+0x39/0x70 [virtio_net]
[ 409.404824] ? virtnet_poll+0x94d/0xc78 [virtio_net]
[ 409.407552] ? receive_buf+0x5120/0x5120 [virtio_net]
[ 409.410255] ? __netif_receive_skb_one_core+0x97/0x1d0
[ 409.413023] ? account_entity_enqueue+0x340/0x4c0
[ 409.415639] __netif_receive_skb_one_core+0x97/0x1d0
[ 409.418346] ? __netif_receive_skb_core+0x2d70/0x2d70
[ 409.421143] ? _raw_write_trylock+0x100/0x100
[ 409.423775] process_backlog+0x19c/0x650
[ 409.426383] net_rx_action+0x71e/0xbc0
[ 409.428960] ? napi_complete_done+0x360/0x360
[ 409.431693] ? handle_irq_event_percpu+0xeb/0x140
[ 409.434705] ? _raw_spin_lock+0x7a/0xd0
[ 409.439881] ? _raw_write_trylock+0x100/0x100
[ 409.444967] __do_softirq+0x1db/0x5f9
[ 409.447999] irq_exit+0x123/0x150
[ 409.451116] do_IRQ+0x71/0x160
[ 409.453709] common_interrupt+0xf/0xf
[ 409.456384] </IRQ>
[ 409.458708] RIP: 0010:native_safe_halt+0xe/0x10
[ 409.461587] Code: 09 f9 fe 48 8b 04 24 e9 12 ff ff ff e9 07 00 00 00 0f 00 2d d4 60 52 00 f4 c3 66 90 e9 07 00 00 00 0f 00 2d c4 60 52 00 fb f4 <c3> 90 66 66 66 66 90 41 56 41 55 41 54 55 53 e8 7e 05 ba fe 65 44
[ 409.469813] RSP: 0018:ffff88805056fd98 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffd3
[ 409.474002] RAX: ffffffff97f1ad00 RBX: 0000000000000002 RCX: ffffffff96abbbd6
[ 409.477775] RDX: 1ffff1100a0a63d0 RSI: 0000000000000004 RDI: ffff8880510b3f38
[ 409.481830] RBP: ffffed100a0a63d0 R08: ffffed100a2167e8 R09: ffffed100a2167e7
[ 409.486087] R10: ffff8880510b3f3b R11: ffffed100a2167e8 R12: ffffffff98e60480
[ 409.490757] R13: 0000000000000002 R14: 0000000000000000 R15: ffff888050531e80
[ 409.495061] ? ldsem_down_write+0x590/0x590
[ 409.499258] ? rcu_idle_enter+0x106/0x150
[ 409.502755] ? tsc_verify_tsc_adjust+0x96/0x2a0
[ 409.506287] default_idle+0x1f/0x280
[ 409.509105] do_idle+0x2d8/0x3e0
[ 409.511741] ? arch_cpu_idle_exit+0x40/0x40
[ 409.514588] cpu_startup_entry+0x19/0x20
[ 409.517355] start_secondary+0x316/0x3f0
[ 409.520051] ? set_cpu_sibling_map+0x19c0/0x19c0
[ 409.522871] secondary_startup_64+0xa4/0xb0
[ 409.525603]
[ 409.527708] Allocated by task 0:
[ 409.530201] save_stack+0x19/0x70
[ 409.532757] __kasan_kmalloc.constprop.6+0xa6/0xf0
[ 409.535660] __kmalloc_node_track_caller+0xe3/0x2c0
[ 409.538641] __kmalloc_reserve.isra.45+0x2e/0xb0
[ 409.541426] pskb_expand_head+0x118/0xcb0
[ 409.544107] __pskb_pull_tail+0xb9/0x1980
[ 409.546749] validate_xmit_skb+0x556/0xab0
[ 409.549387] __dev_queue_xmit+0x11bc/0x2b70
[ 409.552145] ip_finish_output2+0xa7e/0x18c0
[ 409.554817] ip_output+0x1a1/0x2b0
[ 409.557335] ip_forward+0xe39/0x1a20
[ 409.559875] ip_rcv+0xb3/0x180
[ 409.562271] __netif_receive_skb_one_core+0x145/0x1d0
[ 409.565068] netif_receive_skb_internal+0x84/0x1a0
[ 409.567841] napi_gro_flush+0x1d7/0x380
[ 409.570492] napi_complete_done+0x172/0x360
[ 409.573147] virtqueue_napi_complete+0x2a/0x70 [virtio_net]
[ 409.576154] virtnet_poll+0x94d/0xc78 [virtio_net]
[ 409.578963] net_rx_action+0x71e/0xbc0
[ 409.581482] __do_softirq+0x1db/0x5f9
[ 409.584023]
[ 409.586052] Freed by task 12968:
[ 409.588489] save_stack+0x19/0x70
[ 409.591065] __kasan_slab_free+0x122/0x180
[ 409.593775] kfree+0xa6/0x1f0
[ 409.596228] consume_skb+0x69/0x160
[ 409.598803] tun_do_read+0x53c/0x1b30 [tun]
[ 409.601563] tun_chr_read_iter+0x14a/0x2d0 [tun]
[ 409.604477] new_sync_read+0x3e3/0x6b0
[ 409.606956] vfs_read+0xe9/0x2d0
[ 409.609283] ksys_read+0xe8/0x1c0
[ 409.611566] do_syscall_64+0xa0/0x2c0
[ 409.613927] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 409.616671]
[ 409.618573] The buggy address belongs to the object at ffff888046e98000
[ 409.618573] which belongs to the cache kmalloc-2k of size 2048
[ 409.624699] The buggy address is located 184 bytes inside of
[ 409.624699] 2048-byte region [ffff888046e98000, ffff888046e98800)
[ 409.630602] The buggy address belongs to the page:
[ 409.633445] page:ffffea00011ba600 refcount:1 mapcount:0 mapping:ffff888050c02a80 index:0x0 compound_mapcount: 0
[ 409.638069] flags: 0xffffc000010200(slab|head)
[ 409.641048] raw: 00ffffc000010200 dead000000000100 dead000000000200 ffff888050c02a80
[ 409.644876] raw: 0000000000000000 00000000000f000f 00000001ffffffff 0000000000000000
[ 409.648486] page dumped because: kasan: bad access detected
[ 409.651569]
[ 409.653718] Memory state around the buggy address:
[ 409.656635] ffff888046e97f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 409.660222] ffff888046e98000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 409.663775] >ffff888046e98080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 409.667332] ^
[ 409.670247] ffff888046e98100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 409.673815] ffff888046e98180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 409.677391] ==================================================================
[ 409.681002] Disabling lock debugging due to kernel taint
^ permalink raw reply related
* Re: [PATCH 3/4] net: dsa: vsc73xx: add support for parallel mode
From: Florian Fainelli @ 2019-07-01 16:45 UTC (permalink / raw)
To: Pawel Dembicki
Cc: linus.walleij, Andrew Lunn, Vivien Didelot, David S. Miller,
Rob Herring, Mark Rutland, netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-3-paweldembicki@gmail.com>
On 7/1/19 8:27 AM, Pawel Dembicki wrote:
> This patch add platform part of vsc73xx driver.
> It allows to use chip connected by PI interface.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---
[snip]
> + struct vsc73xx_platform *vsc_platform = vsc->priv;
> + u32 offset;
> +
> + if (!vsc73xx_is_addr_valid(block, subblock))
> + return -EINVAL;
> +
> + offset = vsc73xx_make_addr(block, subblock, reg);
> +
> + mutex_lock(&vsc->lock);
> + iowrite32be(val, vsc_platform->base_addr + offset);
> + mutex_unlock(&vsc->lock);
Similar question from Andrew, why is the locking done in the platform
layer, should not that be done in the core I/O operation instead?
> +
> + return 0;
> +}
> +
> +static int vsc73xx_platform_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct vsc73xx_platform *vsc_platform;
> + struct resource *res = NULL;
> + int ret;
> +
> + vsc_platform = devm_kzalloc(dev, sizeof(*vsc_platform), GFP_KERNEL);
> + if (!vsc_platform)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, vsc_platform);
> + vsc_platform->pdev = pdev;
> + vsc_platform->vsc.dev = dev;
> + vsc_platform->vsc.priv = vsc_platform;
> + vsc_platform->vsc.ops = &vsc73xx_platform_ops;
> +
> + /* obtain I/O memory space */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "cannot obtain I/O memory space\n");
> + ret = -ENXIO;
> + return ret;
> + }
> +
> + vsc_platform->base_addr = devm_ioremap_resource(&pdev->dev, res);
devm_ioremap_resource takes care of checking that the resource pointer
is valid, no need to do that here.
> + if (!vsc_platform->base_addr) {
if (IS_ERR(vsc_platform->base_addr))
--
Florian
^ permalink raw reply
* Re: [PATCH 1/4] net: dsa: Change DT bindings for Vitesse VSC73xx switches
From: Florian Fainelli @ 2019-07-01 16:44 UTC (permalink / raw)
To: Pawel Dembicki
Cc: linus.walleij, Andrew Lunn, Vivien Didelot, David S. Miller,
Rob Herring, Mark Rutland, netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-1-paweldembicki@gmail.com>
On 7/1/19 8:27 AM, Pawel Dembicki wrote:
> This commit document changes after split vsc73xx driver into core and
> spi part. The change of DT bindings is required for support the same
> vsc73xx chip, which need PI bus to communicate with CPU. It also
> introduce how to use vsc73xx platform driver.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---
> .../bindings/net/dsa/vitesse,vsc73xx.txt | 74 ++++++++++++++++---
> 1 file changed, 64 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt b/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
> index ed4710c40641..c6a4cd85891c 100644
> --- a/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
> @@ -2,8 +2,8 @@ Vitesse VSC73xx Switches
> ========================
>
> This defines device tree bindings for the Vitesse VSC73xx switch chips.
> -The Vitesse company has been acquired by Microsemi and Microsemi in turn
> -acquired by Microchip but retains this vendor branding.
> +The Vitesse company has been acquired by Microsemi and Microsemi has
> +been acquired Microchip but retains this vendor branding.
>
> The currently supported switch chips are:
> Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
> @@ -11,16 +11,26 @@ Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
> Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
> Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
>
> -The device tree node is an SPI device so it must reside inside a SPI bus
> -device tree node, see spi/spi-bus.txt
> +This switch could have two different management interface.
> +
> +If SPI interface is used, the device tree node is an SPI device so it must
> +reside inside a SPI bus device tree node, see spi/spi-bus.txt
> +
> +If Platform driver is used, the device tree node is an platform device so it
> +must reside inside a platform bus device tree node.
That should not be required because the device remains the same and how
it connects to the host is entirely described by the Device Tree topology.
Take b53 for instance which supports MDIO and SPI by default, and
optionally memory mapped and SRAB (indirect memory map) accesses, they
all have the same compatible strings. Whether the switches will appear
as spi_device, platform_device, or something else is entirely based on
how the Device Tree is laid out.
--
Florian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox