2018-05-18 21:24 GMT+01:00 Daniel Borkmann [off-list ref]:

quoted

+#define MAX_PROG_NAME 256
+static const struct nla_policy bpf_prog_policy[LWT_BPF_PROG_MAX + 1] = {
+     [LWT_BPF_PROG_FD]   = { .type = NLA_U32, },

From UAPI point of view, I wouldn't name it LWT_BPF_PROG_FD but rather something like
LWT_BPF_PROG for example. That way, the setup can contain the fd number, but on the
dump you can put the prog->aux->id in there so that prog lookup can be done again.

Good idea.

quoted

+     [LWT_BPF_PROG_NAME] = { .type = NLA_NUL_STRING,
+                             .len = MAX_PROG_NAME },
+};
+
+static int parse_nla_bpf(struct nlattr **attrs, struct seg6_local_lwt *slwt)
+{
+     struct nlattr *tb[LWT_BPF_PROG_MAX + 1];
+     struct bpf_prog *p;
+     int ret;
+     u32 fd;
+
+     ret = nla_parse_nested(tb, LWT_BPF_PROG_MAX, attrs[SEG6_LOCAL_BPF],
+                            bpf_prog_policy, NULL);
+     if (ret < 0)
+             return ret;
+
+     if (!tb[LWT_BPF_PROG_FD] || !tb[LWT_BPF_PROG_NAME])
+             return -EINVAL;
+
+     slwt->bpf.name = nla_memdup(tb[LWT_BPF_PROG_NAME], GFP_KERNEL);
+     if (!slwt->bpf.name)
+             return -ENOMEM;
+
+     fd = nla_get_u32(tb[LWT_BPF_PROG_FD]);
+     p = bpf_prog_get_type(fd, BPF_PROG_TYPE_LWT_SEG6LOCAL);
+     if (IS_ERR(p))
+             return PTR_ERR(p);

Here in the above error path is definitely a bug in that you don't free the
prior allocated slwt->bpf.name from nla_memdup().

Indeed, I took this part from the lwt bpf infrastructure. I sent a
patch to fix it there too.

Also when you destroy the struct seg6_local_lwt object, what I'm not getting
is where you drop the prog reference again and free slwt->bpf.name there?

I totally missed this. Sending a v7 with all of this fixed.

Thanks for your comments !