| 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
| ||
|
Date: Fri, 29 Jun 2018 17:24:59 -0700
From: Tushar Dave <tushar.n.dave@...cle.com>
To: Daniel Borkmann <daniel@...earbox.net>, ast@...nel.org,
davem@...emloft.net, jakub.kicinski@...ronome.com,
quentin.monnet@...ronome.com, jiong.wang@...ronome.com,
guro@...com, sandipan@...ux.vnet.ibm.com, john.fastabend@...il.com,
kafai@...com, rdna@...com, brakmo@...com, netdev@...r.kernel.org,
acme@...hat.com, sowmini.varadhan@...cle.com
Subject: Re: [RFC v2 PATCH 2/4] ebpf: Add sg_filter_run and sg helper
On 06/29/2018 01:18 AM, Daniel Borkmann wrote:
> On 06/19/2018 08:00 PM, Tushar Dave wrote:
>> When sg_filter_run() is invoked it runs the attached eBPF
>> SOCKET_SG_FILTER program which deals with struct scatterlist.
>>
>> In addition, this patch also adds bpf_sg_next helper function that
>> allows users to retrieve the next sg element from sg list.
>>
>> Signed-off-by: Tushar Dave <tushar.n.dave@...cle.com>
>> Acked-by: Sowmini Varadhan <sowmini.varadhan@...cle.com>
>> ---
>> include/linux/filter.h | 2 +
>> include/uapi/linux/bpf.h | 10 ++++-
>> net/core/filter.c | 72 +++++++++++++++++++++++++++++++
>> tools/include/uapi/linux/bpf.h | 10 ++++-
>> tools/testing/selftests/bpf/bpf_helpers.h | 3 ++
>> 5 files changed, 95 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index 71618b1..d176402 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> @@ -1072,4 +1072,6 @@ struct bpf_sock_ops_kern {
>> */
>> };
>>
>> +int sg_filter_run(struct sock *sk, struct scatterlist *sg);
>> +
>> #endif /* __LINUX_FILTER_H__ */
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index ef0a7b6..036432b 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -2076,6 +2076,13 @@ struct bpf_stack_build_id {
>> * Return
>> * A 64-bit integer containing the current cgroup id based
>> * on the cgroup within which the current task is running.
>> + *
>> + * int bpf_sg_next(struct bpf_scatterlist *sg)
>> + * Description
>> + * This helper allows user to retrieve next sg element from
>> + * sg list.
>> + * Return
>> + * Returns 0 on success, or a negative error in case of failure.
>> */
>> #define __BPF_FUNC_MAPPER(FN) \
>> FN(unspec), \
>> @@ -2158,7 +2165,8 @@ struct bpf_stack_build_id {
>> FN(rc_repeat), \
>> FN(rc_keydown), \
>> FN(skb_cgroup_id), \
>> - FN(get_current_cgroup_id),
>> + FN(get_current_cgroup_id), \
>> + FN(sg_next),
>>
>> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>> * function eBPF program intends to call
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 8f67942..702ff5b 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -121,6 +121,53 @@ int sk_filter_trim_cap(struct sock *sk, struct sk_buff *skb, unsigned int cap)
>> }
>> EXPORT_SYMBOL(sk_filter_trim_cap);
>>
>> +int sg_filter_run(struct sock *sk, struct scatterlist *sg)
>> +{
>> + struct sk_filter *filter;
>> + int err;
>> +
>> + rcu_read_lock();
>> + filter = rcu_dereference(sk->sk_filter);
>> + if (filter) {
>> + struct bpf_scatterlist bpfsg;
>> + int num_sg;
>> +
>> + if (!sg) {
>> + err = -EINVAL;
>> + goto out;
>> + }
>> +
>> + num_sg = sg_nents(sg);
>> + if (num_sg <= 0) {
>> + err = -EINVAL;
>> + goto out;
>> + }
>> +
>> + /* We store a reference to the sg list so it can later used by
>> + * eBPF helpers to retrieve the next sg element.
>> + */
>> + bpfsg.num_sg = num_sg;
>> + bpfsg.cur_sg = 0;
>> + bpfsg.sg = sg;
>> +
>> + /* For the first sg element, we store the pkt access pointers
>> + * into start and end so eBPF program can have pkt access using
>> + * data and data_end. The pkt access for subsequent element of
>> + * sg list is possible when eBPF program invokes bpf_sg_next
>> + * which takes care of setting start and end to the correct sg
>> + * element.
>> + */
>> + bpfsg.start = sg_virt(sg);
>> + bpfsg.end = bpfsg.start + sg->length;
>> + BPF_PROG_RUN(filter->prog, &bpfsg);
>> + }
>> +out:
>> + rcu_read_unlock();
>> +
>> + return err;
>> +}
>> +EXPORT_SYMBOL(sg_filter_run);
>> +
>> BPF_CALL_1(bpf_skb_get_pay_offset, struct sk_buff *, skb)
>> {
>> return skb_get_poff(skb);
>> @@ -3753,6 +3800,29 @@ static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
>> .arg1_type = ARG_PTR_TO_CTX,
>> };
>>
>> +BPF_CALL_1(bpf_sg_next, struct bpf_scatterlist *, bpfsg)
>> +{
>> + struct scatterlist *sg = bpfsg->sg;
>> + int cur_sg = bpfsg->cur_sg;
>> +
>> + cur_sg++;
>> + if (cur_sg >= bpfsg->num_sg)
>> + return -ENODATA;
>> +
>> + bpfsg->cur_sg = cur_sg;
>> + bpfsg->start = sg_virt(&sg[cur_sg]);
>> + bpfsg->end = bpfsg->start + sg[cur_sg].length;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct bpf_func_proto bpf_sg_next_proto = {
>> + .func = bpf_sg_next,
>> + .gpl_only = false,
>> + .ret_type = RET_INTEGER,
>> + .arg1_type = ARG_PTR_TO_CTX,
>> +};
>
> Should be added to bpf_helper_changes_pkt_data() in order to enforce a reload
> of all pkt pointers. Otherwise this is buggy in the sense that someone could only
> reload pkt_end pointer in the prog while old pkt_start still points to previous
> sg entry, so you would be able to access out of bounds.
Sure thing. Will do so.
Thank you.
-Tushar
>
> Thanks,
> Daniel
>
Powered by blists - more mailing lists