All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Martin KaFai Lau <kafai@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Clark Williams <williams@redhat.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH bpf 1/2] bpf: enforce that struct_ops programs be GPL-only
Date: Thu, 25 Mar 2021 21:37:23 +0100	[thread overview]
Message-ID: <87blb7dl6k.fsf@toke.dk> (raw)
In-Reply-To: <20210325181552.ottuv7shqgfwxlsg@kafai-mbp.dhcp.thefacebook.com>

Martin KaFai Lau <kafai@fb.com> writes:

> On Thu, Mar 25, 2021 at 04:40:33PM +0100, Toke Høiland-Jørgensen wrote:
>> With the introduction of the struct_ops program type, it became possible to
>> implement kernel functionality in BPF, making it viable to use BPF in place
>> of a regular kernel module for these particular operations.
>> 
>> Thus far, the only user of this mechanism is for implementing TCP
>> congestion control algorithms. These are clearly marked as GPL-only when
>> implemented as modules (as seen by the use of EXPORT_SYMBOL_GPL for
>> tcp_register_congestion_control()), so it seems like an oversight that this
>> was not carried over to BPF implementations. And sine this is the only user
>> of the struct_ops mechanism, just enforcing GPL-only for the struct_ops
>> program type seems like the simplest way to fix this.
>> 
>> Fixes: 0baf26b0fcd7 ("bpf: tcp: Support tcp_congestion_ops in bpf")
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> ---
>>  kernel/bpf/verifier.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 44e4ec1640f1..48dd0c0f087c 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -12166,6 +12166,11 @@ static int check_struct_ops_btf_id(struct bpf_verifier_env *env)
>>  		return -ENOTSUPP;
>>  	}
>>  
>> +	if (!prog->gpl_compatible) {
>> +		verbose(env, "struct ops programs must have a GPL compatible license\n");
>> +		return -EINVAL;
>> +	}
>> +
> Thanks for the patch.
>
> A nit.  Instead of sitting in between of the attach_btf_id check
> and expected_attach_type check, how about moving it to the beginning
> of this function.  Checking attach_btf_id and expected_attach_type
> would make more sense to be done next to each other as in the current
> code.

Yeah, good point. Not sure what I was thinking stuffing it in the middle
there; will fix and send a v2!

> Acked-by: Martin KaFai Lau <kafai@fb.com>

Thanks! :)

-Toke


      reply	other threads:[~2021-03-25 20:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-25 15:40 [PATCH bpf 1/2] bpf: enforce that struct_ops programs be GPL-only Toke Høiland-Jørgensen
2021-03-25 15:40 ` [PATCH bpf 2/2] bpf/selftests: test that kernel rejects a TCP CC with an invalid license Toke Høiland-Jørgensen
2021-03-25 18:32   ` Martin KaFai Lau
2021-03-25 20:38     ` Toke Høiland-Jørgensen
2021-03-25 18:15 ` [PATCH bpf 1/2] bpf: enforce that struct_ops programs be GPL-only Martin KaFai Lau
2021-03-25 20:37   ` Toke Høiland-Jørgensen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87blb7dl6k.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sdf@google.com \
    --cc=songliubraving@fb.com \
    --cc=williams@redhat.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.