netdev - Re: [PATCH net-next v2 05/15] xdp: allow attaching programs loaded for specific device

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] [day] [month] [year] [list]

Message-ID: <935db219-863a-3164-6b60-b203eeafa5ae@iogearbox.net>
Date:   Sun, 12 Nov 2017 20:33:07 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jiri Pirko <jiri@...nulli.us>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     netdev@...r.kernel.org, oss-drivers@...ronome.com,
        alexei.starovoitov@...il.com
Subject: Re: [PATCH net-next v2 05/15] xdp: allow attaching programs loaded
 for specific device

On 11/12/2017 10:00 AM, Jiri Pirko wrote:
> Fri, Nov 03, 2017 at 09:56:20PM CET, jakub.kicinski@...ronome.com wrote:
>> Pass the netdev pointer to bpf_prog_get_type().  This way
>> BPF code can decide whether the device matches what the
>> code was loaded/translated for.
> 
> [...]
> 
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 3217c20ea91b..68f9123acd39 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -1057,7 +1057,22 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)
>> }
>> EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
>>
>> -static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)
>> +static bool bpf_prog_can_attach(struct bpf_prog *prog,
>> +				enum bpf_prog_type *attach_type,
>> +				struct net_device *netdev)
>> +{
>> +	struct bpf_dev_offload *offload = prog->aux->offload;
>> +
>> +	if (prog->type != *attach_type)
>> +		return false;
>> +	if (offload && offload->netdev != netdev)
> 
> This means you return false in case netdev function arg is NULL. Is that
> intentional?
> 
> Seems wrong to me because for example in cls_bpf_prog_from_efd, you
> would get an error in case TCA_CLS_FLAGS_SKIP_SW is not set.

I think it's fine, the prog was originally loaded in the verifier pass
to be JITed via nfp JIT, so you'll have a valid prog->aux->offload, and
you're specifying TCA_CLS_FLAGS_SKIP_SW for fetching the qdisc dev where
we match devs above. So if that dev is not set (NULL) e.g. due to intention
of running the specified prog in sw, then you cannot use that bpf_prog
which was loaded as offloaded one instead. Looks correct to me.