netdev - Re: [PATCH bpf-next v2 3/6] bpf: support attaching freplace programs to multiple attach points

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 15 Jul 2020 13:44:06 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 3/6] bpf: support attaching freplace programs
 to multiple attach points

On Wed, Jul 15, 2020 at 03:09:02PM +0200, Toke Høiland-Jørgensen wrote:
>  
> +	if (tgt_prog_fd) {
> +		/* For now we only allow new targets for BPF_PROG_TYPE_EXT */
> +		if (prog->type != BPF_PROG_TYPE_EXT ||
> +		    !btf_id) {
> +			err = -EINVAL;
> +			goto out_put_prog;
> +		}
> +		tgt_prog = bpf_prog_get(tgt_prog_fd);
> +		if (IS_ERR(tgt_prog)) {
> +			err = PTR_ERR(tgt_prog);
> +			tgt_prog = NULL;
> +			goto out_put_prog;
> +		}
> +
> +	} else if (btf_id) {
> +		err = -EINVAL;
> +		goto out_put_prog;
> +	} else {
> +		btf_id = prog->aux->attach_btf_id;
> +		tgt_prog = prog->aux->linked_prog;
> +		if (tgt_prog)
> +			bpf_prog_inc(tgt_prog); /* we call bpf_prog_put() on link release */

so the first prog_load cmd will beholding the first target prog?
This is complete non starter.
You didn't mention such decision anywhere.
The first ext prog will attach to the first dispatcher xdp prog,
then that ext prog will multi attach to second dispatcher xdp prog and
the first dispatcher prog will live in the kernel forever.
That's not what we discussed back in April.

> +	}
> +	err = bpf_check_attach_target(NULL, prog, tgt_prog, btf_id,
> +				      &fmodel, &addr, NULL, NULL);

This is a second check for btf id match?
What's the point? The first one was done at load time.
When tgt_prog_fd/tgt_btf_id are zero there is no need to recheck.

I really hope I'm misreading these patches, because they look very raw.