Topics

bpf_redirect_map not working after tail call

Sebastiano Miano
 

Dear all,

We have noticed that the bpf_redirect_map returns an error when it is
called after a tail call.
The xdp_redirect_map program (under sample/bpf) works fine, but if we
modify it as shown in the following diff, it doesn't work anymore.
I have debugged it with the xdp_monitor application and the error
returned is EFAULT.
Is this a known issue? Am I doing something wrong?

Thanks,
Sebastiano

P.S. I have tested the program with the latest bpf-next kernel.

------------

diff --git a/samples/bpf/xdp_redirect_map_kern.c
b/samples/bpf/xdp_redirect_map_kern.c
index 740a529..bf1275a 100644
--- a/samples/bpf/xdp_redirect_map_kern.c
+++ b/samples/bpf/xdp_redirect_map_kern.c
@@ -36,6 +36,13 @@ struct bpf_map_def SEC("maps") rxcnt = {
.max_entries = 1,
};

+struct bpf_map_def SEC("maps") prog_table = {
+ .type = BPF_MAP_TYPE_PROG_ARRAY,
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .max_entries = 32,
+};
+
static void swap_src_dst_mac(void *data)
{
unsigned short *p = data;
@@ -89,4 +96,15 @@ int xdp_redirect_dummy_prog(struct xdp_md *ctx)
return XDP_PASS;
}

+/* Entry point */
+SEC("xdp_redirect_entry_point")
+int xdp_redirect_entry_point_prog(struct xdp_md *ctx)
+{
+ //char fmt[] = "xdp_redirect_entry_point\n";
+ //bpf_trace_printk(fmt, sizeof(fmt));
+ bpf_tail_call(ctx, &prog_table, 0);
+ // Tail call failed
+ return XDP_DROP;
+}
+
char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/xdp_redirect_map_user.c
b/samples/bpf/xdp_redirect_map_user.c
index 4445e76..b2d2059 100644
--- a/samples/bpf/xdp_redirect_map_user.c
+++ b/samples/bpf/xdp_redirect_map_user.c
@@ -120,7 +120,13 @@ int main(int argc, char **argv)
return 1;
}

- if (bpf_set_link_xdp_fd(ifindex_in, prog_fd[0], xdp_flags) < 0) {
+ ret = bpf_map_update_elem(map_fd[2], &key, &prog_fd[0], 0);
+ if (ret) {
+ perror("bpf_update_elem");
+ goto out;
+ }
+
+ if (bpf_set_link_xdp_fd(ifindex_in, prog_fd[2], xdp_flags) < 0) {
printf("ERROR: link set xdp fd failed on %d\n", ifindex_in);
return 1;
}

Jesper Dangaard Brouer
 

On Fri, 1 Jun 2018 14:15:58 +0200
Sebastiano Miano via iovisor-dev <iovisor-dev@...> wrote:
Dear all,

We have noticed that the bpf_redirect_map returns an error when it is
called after a tail call.
The xdp_redirect_map program (under sample/bpf) works fine, but if we
modify it as shown in the following diff, it doesn't work anymore.
I have debugged it with the xdp_monitor application and the error
returned is EFAULT.
Is this a known issue? Am I doing something wrong?
Argh, this is likely an issue/bug due to the check xdp_map_invalid(),
that was introduced in commit 7c3001313396 ("bpf: fix ri->map_owner
pointer on bpf_prog_realloc").

To Daniel, I don't know how to solve this, could you give some advice?



static inline bool xdp_map_invalid(const struct bpf_prog *xdp_prog,
unsigned long aux)
{
return (unsigned long)xdp_prog->aux != aux;
}

static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
unsigned long map_owner = ri->map_owner;
struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
void *fwd = NULL;
int err;

[...]
if (unlikely(xdp_map_invalid(xdp_prog, map_owner))) {
err = -EFAULT;
map = NULL;
goto err;
}
[...]

P.S. I have tested the program with the latest bpf-next kernel.

------------

diff --git a/samples/bpf/xdp_redirect_map_kern.c
b/samples/bpf/xdp_redirect_map_kern.c
index 740a529..bf1275a 100644
--- a/samples/bpf/xdp_redirect_map_kern.c
+++ b/samples/bpf/xdp_redirect_map_kern.c
@@ -36,6 +36,13 @@ struct bpf_map_def SEC("maps") rxcnt = {
.max_entries = 1,
};

+struct bpf_map_def SEC("maps") prog_table = {
+ .type = BPF_MAP_TYPE_PROG_ARRAY,
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .max_entries = 32,
+};
+
static void swap_src_dst_mac(void *data)
{
unsigned short *p = data;
@@ -89,4 +96,15 @@ int xdp_redirect_dummy_prog(struct xdp_md *ctx)
return XDP_PASS;
}

+/* Entry point */
+SEC("xdp_redirect_entry_point")
+int xdp_redirect_entry_point_prog(struct xdp_md *ctx)
+{
+ //char fmt[] = "xdp_redirect_entry_point\n";
+ //bpf_trace_printk(fmt, sizeof(fmt));
+ bpf_tail_call(ctx, &prog_table, 0);
+ // Tail call failed
+ return XDP_DROP;
+}
+
char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/xdp_redirect_map_user.c
b/samples/bpf/xdp_redirect_map_user.c
index 4445e76..b2d2059 100644
--- a/samples/bpf/xdp_redirect_map_user.c
+++ b/samples/bpf/xdp_redirect_map_user.c
@@ -120,7 +120,13 @@ int main(int argc, char **argv)
return 1;
}

- if (bpf_set_link_xdp_fd(ifindex_in, prog_fd[0], xdp_flags) < 0) {
+ ret = bpf_map_update_elem(map_fd[2], &key, &prog_fd[0], 0);
+ if (ret) {
+ perror("bpf_update_elem");
+ goto out;
+ }
+
+ if (bpf_set_link_xdp_fd(ifindex_in, prog_fd[2], xdp_flags) < 0) {
printf("ERROR: link set xdp fd failed on %d\n", ifindex_in);
return 1;
}
_______________________________________________
iovisor-dev mailing list
iovisor-dev@...
https://lists.iovisor.org/mailman/listinfo/iovisor-dev


--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer

Daniel Borkmann
 

On 06/04/2018 01:04 PM, Jesper Dangaard Brouer via iovisor-dev wrote:
On Fri, 1 Jun 2018 14:15:58 +0200
Sebastiano Miano via iovisor-dev <iovisor-dev@...> wrote:

Dear all,

We have noticed that the bpf_redirect_map returns an error when it is
called after a tail call.
The xdp_redirect_map program (under sample/bpf) works fine, but if we
modify it as shown in the following diff, it doesn't work anymore.
I have debugged it with the xdp_monitor application and the error
returned is EFAULT.
Is this a known issue? Am I doing something wrong?
Argh, this is likely an issue/bug due to the check xdp_map_invalid(),
that was introduced in commit 7c3001313396 ("bpf: fix ri->map_owner
pointer on bpf_prog_realloc").

To Daniel, I don't know how to solve this, could you give some advice?



static inline bool xdp_map_invalid(const struct bpf_prog *xdp_prog,
unsigned long aux)
{
return (unsigned long)xdp_prog->aux != aux;
}

static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
unsigned long map_owner = ri->map_owner;
struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
void *fwd = NULL;
int err;

[...]
if (unlikely(xdp_map_invalid(xdp_prog, map_owner))) {
err = -EFAULT;
map = NULL;
goto err;
}
[...]
Argh, I see the issue. Working on a fix after checking the syzkaller reports.

Thanks for the report!