* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Michael S. Tsirkin @ 2018-05-03 1:44 UTC (permalink / raw)
To: Tiwei Bie
Cc: Jason Wang, virtualization, linux-kernel, netdev, wexu, jfreimann
In-Reply-To: <20180503011116.qvoyblcpklinrk26@debian>
On Thu, May 03, 2018 at 09:11:16AM +0800, Tiwei Bie wrote:
> On Wed, May 02, 2018 at 06:42:57PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 02, 2018 at 11:12:55PM +0800, Tiwei Bie wrote:
> > > On Wed, May 02, 2018 at 04:51:01PM +0300, Michael S. Tsirkin wrote:
> > > > On Wed, May 02, 2018 at 03:28:19PM +0800, Tiwei Bie wrote:
> > > > > On Wed, May 02, 2018 at 10:51:06AM +0800, Jason Wang wrote:
> > > > > > On 2018年04月25日 13:15, Tiwei Bie wrote:
> > > > > > > This commit introduces the event idx support in packed
> > > > > > > ring. This feature is temporarily disabled, because the
> > > > > > > implementation in this patch may not work as expected,
> > > > > > > and some further discussions on the implementation are
> > > > > > > needed, e.g. do we have to check the wrap counter when
> > > > > > > checking whether a kick is needed?
> > > > > > >
> > > > > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > > > > ---
> > > > > > > drivers/virtio/virtio_ring.c | 53 ++++++++++++++++++++++++++++++++++++++++----
> > > > > > > 1 file changed, 49 insertions(+), 4 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > > > > index 0181e93897be..b1039c2985b9 100644
> > > > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > > > @@ -986,7 +986,7 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
> > > > > > > static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
> > > > > > > {
> > > > > > > struct vring_virtqueue *vq = to_vvq(_vq);
> > > > > > > - u16 flags;
> > > > > > > + u16 new, old, off_wrap, flags;
> > > > > > > bool needs_kick;
> > > > > > > u32 snapshot;
> > > > > > > @@ -995,7 +995,12 @@ static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
> > > > > > > * suppressions. */
> > > > > > > virtio_mb(vq->weak_barriers);
> > > > > > > + old = vq->next_avail_idx - vq->num_added;
> > > > > > > + new = vq->next_avail_idx;
> > > > > > > + vq->num_added = 0;
> > > > > > > +
> > > > > > > snapshot = *(u32 *)vq->vring_packed.device;
> > > > > > > + off_wrap = virtio16_to_cpu(_vq->vdev, snapshot & 0xffff);
> > > > > > > flags = cpu_to_virtio16(_vq->vdev, snapshot >> 16) & 0x3;
> > > > > > > #ifdef DEBUG
> > > > > > > @@ -1006,7 +1011,10 @@ static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
> > > > > > > vq->last_add_time_valid = false;
> > > > > > > #endif
> > > > > > > - needs_kick = (flags != VRING_EVENT_F_DISABLE);
> > > > > > > + if (flags == VRING_EVENT_F_DESC)
> > > > > > > + needs_kick = vring_need_event(off_wrap & ~(1<<15), new, old);
> > > > > >
> > > > > > I wonder whether or not the math is correct. Both new and event are in the
> > > > > > unit of descriptor ring size, but old looks not.
> > > > >
> > > > > What vring_need_event() cares is the distance between
> > > > > `new` and `old`, i.e. vq->num_added. So I think there
> > > > > is nothing wrong with `old`. But the calculation of the
> > > > > distance between `new` and `event_idx` isn't right when
> > > > > `new` wraps. How do you think about the below code:
> > > > >
> > > > > wrap_counter = off_wrap >> 15;
> > > > > event_idx = off_wrap & ~(1<<15);
> > > > > if (wrap_counter != vq->wrap_counter)
> > > > > event_idx -= vq->vring_packed.num;
> > > > >
> > > > > needs_kick = vring_need_event(event_idx, new, old);
> > > >
> > > > I suspect this hack won't work for non power of 2 ring.
> > >
> > > Above code doesn't require the ring size to be a power of 2.
> > >
> > > For (__u16)(new_idx - old), what we want to get is vq->num_added.
> > >
> > > old = vq->next_avail_idx - vq->num_added;
> > > new = vq->next_avail_idx;
> > >
> > > When vq->next_avail_idx >= vq->num_added, it's obvious that,
> > > (__u16)(new_idx - old) is vq->num_added.
> > >
> > > And when vq->next_avail_idx < vq->num_added, new will be smaller
> > > than old (old will be a big unsigned number), but (__u16)(new_idx
> > > - old) is still vq->num_added.
> > >
> > > For (__u16)(new_idx - event_idx - 1), when new wraps and event_idx
> > > doesn't wrap, the most straightforward way to calculate it is:
> > > (new + vq->vring_packed.num) - event_idx - 1.
> >
> > So how about we use the straightforward way then?
>
> You mean we do new += vq->vring_packed.num instead
> of event_idx -= vq->vring_packed.num before calling
> vring_need_event()?
>
> The problem is that, the second param (new_idx) of
> vring_need_event() will be used for:
>
> (__u16)(new_idx - event_idx - 1)
> (__u16)(new_idx - old)
>
> So if we change new, we will need to change old too.
I think that since we have a branch there anyway,
we are better off just special-casing if (wrap_counter != vq->wrap_counter).
Treat is differenty and avoid casts.
> And that would be an ugly hack..
>
> Best regards,
> Tiwei Bie
I consider casts and huge numbers with two's complement
games even uglier.
> >
> > > But we can also calculate it in this way:
> > >
> > > event_idx -= vq->vring_packed.num;
> > > (event_idx will be a big unsigned number)
> > >
> > > Then (__u16)(new_idx - event_idx - 1) will be the value we want.
> > >
> > > Best regards,
> > > Tiwei Bie
> >
> >
> > > >
> > > >
> > > > > Best regards,
> > > > > Tiwei Bie
> > > > >
> > > > >
> > > > > >
> > > > > > Thanks
> > > > > >
> > > > > > > + else
> > > > > > > + needs_kick = (flags != VRING_EVENT_F_DISABLE);
> > > > > > > END_USE(vq);
> > > > > > > return needs_kick;
> > > > > > > }
> > > > > > > @@ -1116,6 +1124,15 @@ static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
> > > > > > > if (vq->last_used_idx >= vq->vring_packed.num)
> > > > > > > vq->last_used_idx -= vq->vring_packed.num;
> > > > > > > + /* If we expect an interrupt for the next entry, tell host
> > > > > > > + * by writing event index and flush out the write before
> > > > > > > + * the read in the next get_buf call. */
> > > > > > > + if (vq->event_flags_shadow == VRING_EVENT_F_DESC)
> > > > > > > + virtio_store_mb(vq->weak_barriers,
> > > > > > > + &vq->vring_packed.driver->off_wrap,
> > > > > > > + cpu_to_virtio16(_vq->vdev, vq->last_used_idx |
> > > > > > > + (vq->wrap_counter << 15)));
> > > > > > > +
> > > > > > > #ifdef DEBUG
> > > > > > > vq->last_add_time_valid = false;
> > > > > > > #endif
> > > > > > > @@ -1143,10 +1160,17 @@ static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq)
> > > > > > > /* We optimistically turn back on interrupts, then check if there was
> > > > > > > * more to do. */
> > > > > > > + /* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
> > > > > > > + * either clear the flags bit or point the event index at the next
> > > > > > > + * entry. Always update the event index to keep code simple. */
> > > > > > > +
> > > > > > > + vq->vring_packed.driver->off_wrap = cpu_to_virtio16(_vq->vdev,
> > > > > > > + vq->last_used_idx | (vq->wrap_counter << 15));
> > > > > > > if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
> > > > > > > virtio_wmb(vq->weak_barriers);
> > > > > > > - vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
> > > > > > > + vq->event_flags_shadow = vq->event ? VRING_EVENT_F_DESC :
> > > > > > > + VRING_EVENT_F_ENABLE;
> > > > > > > vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
> > > > > > > vq->event_flags_shadow);
> > > > > > > }
> > > > > > > @@ -1172,15 +1196,34 @@ static bool virtqueue_poll_packed(struct virtqueue *_vq, unsigned last_used_idx)
> > > > > > > static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq)
> > > > > > > {
> > > > > > > struct vring_virtqueue *vq = to_vvq(_vq);
> > > > > > > + u16 bufs, used_idx, wrap_counter;
> > > > > > > START_USE(vq);
> > > > > > > /* We optimistically turn back on interrupts, then check if there was
> > > > > > > * more to do. */
> > > > > > > + /* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
> > > > > > > + * either clear the flags bit or point the event index at the next
> > > > > > > + * entry. Always update the event index to keep code simple. */
> > > > > > > +
> > > > > > > + /* TODO: tune this threshold */
> > > > > > > + bufs = (u16)(vq->next_avail_idx - vq->last_used_idx) * 3 / 4;
> > > > > > > +
> > > > > > > + used_idx = vq->last_used_idx + bufs;
> > > > > > > + wrap_counter = vq->wrap_counter;
> > > > > > > +
> > > > > > > + if (used_idx >= vq->vring_packed.num) {
> > > > > > > + used_idx -= vq->vring_packed.num;
> > > > > > > + wrap_counter ^= 1;
> > > > > > > + }
> > > > > > > +
> > > > > > > + vq->vring_packed.driver->off_wrap = cpu_to_virtio16(_vq->vdev,
> > > > > > > + used_idx | (wrap_counter << 15));
> > > > > > > if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
> > > > > > > virtio_wmb(vq->weak_barriers);
> > > > > > > - vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
> > > > > > > + vq->event_flags_shadow = vq->event ? VRING_EVENT_F_DESC :
> > > > > > > + VRING_EVENT_F_ENABLE;
> > > > > > > vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
> > > > > > > vq->event_flags_shadow);
> > > > > > > }
> > > > > > > @@ -1822,8 +1865,10 @@ void vring_transport_features(struct virtio_device *vdev)
> > > > > > > switch (i) {
> > > > > > > case VIRTIO_RING_F_INDIRECT_DESC:
> > > > > > > break;
> > > > > > > +#if 0
> > > > > > > case VIRTIO_RING_F_EVENT_IDX:
> > > > > > > break;
> > > > > > > +#endif
> > > > > > > case VIRTIO_F_VERSION_1:
> > > > > > > break;
> > > > > > > case VIRTIO_F_IOMMU_PLATFORM:
> > > > > >
^ permalink raw reply
* Re: [PATCH bpf-next 07/12] bpf, sparc64: remove ld_abs/ld_ind
From: David Miller @ 2018-05-03 1:39 UTC (permalink / raw)
To: daniel; +Cc: ast, netdev
In-Reply-To: <20180503010536.7917-8-daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Thu, 3 May 2018 03:05:31 +0200
> Since LD_ABS/LD_IND instructions are now removed from the core and
> reimplemented through a combination of inlined BPF instructions and
> a slow-path helper, we can get rid of the complexity from sparc64 JIT.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: David S. Miller <davem@davemloft.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* [PATCH net-next] ip6_gre: correct the function name in ip6gre_tnl_addr_conflict() comment
From: Sun Lianwen @ 2018-05-03 1:34 UTC (permalink / raw)
To: davem; +Cc: netdev
The function name is wrong in ip6gre_tnl_addr_conflict() comment, which
use ip6_tnl_addr_conflict instead of ip6gre_tnl_addr_conflict.
Signed-off-by: Sun Lianwen <sunlw.fnst@cn.fujitsu.com>
---
net/ipv6/ip6_gre.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 69727bc168cb..4e111da8d453 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -807,7 +807,7 @@ static inline int ip6gre_xmit_ipv6(struct sk_buff *skb, struct net_device *dev)
}
/**
- * ip6_tnl_addr_conflict - compare packet addresses to tunnel's own
+ * ip6gre_tnl_addr_conflict - compare packet addresses to tunnel's own
* @t: the outgoing tunnel device
* @hdr: IPv6 header from the incoming packet
*
--
2.17.0
^ permalink raw reply related
* Re: [PATCH] sctp: fix a potential missing-check bug
From: Wenwen Wang @ 2018-05-03 1:27 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Kangjie Lu, Vlad Yasevich, Neil Horman, David S. Miller,
open list:SCTP PROTOCOL, open list:NETWORKING [GENERAL],
open list, Wenwen Wang
In-Reply-To: <20180503012402.GK5105@localhost.localdomain>
On Wed, May 2, 2018 at 8:24 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Wed, May 02, 2018 at 08:15:45PM -0500, Wenwen Wang wrote:
>> In sctp_setsockopt_maxseg(), the integer 'val' is compared against min_len
>> and max_len to check whether it is in the appropriate range. If it is not,
>> an error code -EINVAL will be returned. This is enforced by a security
>> check. But, this check is only executed when 'val' is not 0. In fact, if
>> 'val' is 0, it will be assigned with a new value (if the return value of
>> the function sctp_id2assoc() is not 0) in the following execution. However,
>> this new value of 'val' is not checked before it is used to assigned to
>> asoc->user_frag. That means it is possible that the new value of 'val'
>> could be out of the expected range. This can cause security issues
>> such as buffer overflows, e.g., the new value of 'val' is used as an index
>> to access a buffer.
>>
>> This patch inserts a check for the new value of 'val' to see if it is in
>> the expected range. If it is not, an error code -EINVAL will be returned.
>>
>> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
>> ---
>> net/sctp/socket.c | 22 +++++++++++-----------
>> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> ?
> This patch is the same as previous one. git send-email <old file>
> maybe?
>
> Marcelo
Thanks for your suggestion, Marcelo. I can send the old file. But, I
have added a line of comment in this patch.
Wenwen
^ permalink raw reply
* Re: [PATCH] sctp: fix a potential missing-check bug
From: Marcelo Ricardo Leitner @ 2018-05-03 1:24 UTC (permalink / raw)
To: Wenwen Wang
Cc: Kangjie Lu, Vlad Yasevich, Neil Horman, David S. Miller,
open list:SCTP PROTOCOL, open list:NETWORKING [GENERAL],
open list
In-Reply-To: <1525310145-28102-1-git-send-email-wang6495@umn.edu>
On Wed, May 02, 2018 at 08:15:45PM -0500, Wenwen Wang wrote:
> In sctp_setsockopt_maxseg(), the integer 'val' is compared against min_len
> and max_len to check whether it is in the appropriate range. If it is not,
> an error code -EINVAL will be returned. This is enforced by a security
> check. But, this check is only executed when 'val' is not 0. In fact, if
> 'val' is 0, it will be assigned with a new value (if the return value of
> the function sctp_id2assoc() is not 0) in the following execution. However,
> this new value of 'val' is not checked before it is used to assigned to
> asoc->user_frag. That means it is possible that the new value of 'val'
> could be out of the expected range. This can cause security issues
> such as buffer overflows, e.g., the new value of 'val' is used as an index
> to access a buffer.
>
> This patch inserts a check for the new value of 'val' to see if it is in
> the expected range. If it is not, an error code -EINVAL will be returned.
>
> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
> ---
> net/sctp/socket.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
?
This patch is the same as previous one. git send-email <old file>
maybe?
Marcelo
^ permalink raw reply
* Re: [PATCH] NET/netlink: optimize output of seq_puts in af_netlink.c
From: YU Bo @ 2018-05-03 1:16 UTC (permalink / raw)
To: David Miller; +Cc: xiyou.wangcong, yuzibode, netdev, kernel-janitors
In-Reply-To: <20180502.101943.1124614050601030061.davem@davemloft.net>
Hi,
On Wed, May 02, 2018 at 10:19:43AM -0400, David Miller wrote:
>From: Bo YU <tsu.yubo@gmail.com>
>Date: Wed, 2 May 2018 05:54:24 -0400
>
>> Optimization of command output: `cat /proc/net/netlink`
>>
>> After the patch, we will get:
>>
>> https://clbin.com/lnu4L
>>
>> Signed-off-by: Bo YU <tsu.yubo@gmail.com>
>> ---
>> net/netlink/af_netlink.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
>> index 55342c4d5cec..2e2dd88fc79f 100644
>> --- a/net/netlink/af_netlink.c
>> +++ b/net/netlink/af_netlink.c
>> @@ -2606,13 +2606,13 @@ static int netlink_seq_show(struct seq_file
>> *seq, void *v)
>> {
>> if (v == SEQ_START_TOKEN) {
>> seq_puts(seq,
>> - "sk Eth Pid Groups "
>> - "Rmem Wmem Dump Locks Drops Inode\n");
>> + "sk Eth Pid Groups "
>> + "Rmem Wmem Dump Locks Drops Inode\n");
>
>Please do not break the indentation of the code like this.
Sorry, i am shame to do like it.There are something happened in my different
version vim.Because checkpatch tell me only
"WARNING: quoted string split across lines"
Thank you, i will fix it.
>
>I wish to unfortunately say, that generally speaking, your patch
>submissions are not of the best quality, and take up a lot of reviewer
>time and resources as a result.
>
>If you do not improve the quality of your submissions, I am giving
>you a kind warning that the amount of care and review your patches
>will receive will become lower. Your submissions might even get to
>the point wheere they are effectively ignored.
>
>So please put more care into your work.
>
>Thank you.
^ permalink raw reply
* [PATCH] sctp: fix a potential missing-check bug
From: Wenwen Wang @ 2018-05-03 1:15 UTC (permalink / raw)
To: Wenwen Wang
Cc: Kangjie Lu, Vlad Yasevich, Neil Horman, David S. Miller,
open list:SCTP PROTOCOL, open list:NETWORKING [GENERAL],
open list
In sctp_setsockopt_maxseg(), the integer 'val' is compared against min_len
and max_len to check whether it is in the appropriate range. If it is not,
an error code -EINVAL will be returned. This is enforced by a security
check. But, this check is only executed when 'val' is not 0. In fact, if
'val' is 0, it will be assigned with a new value (if the return value of
the function sctp_id2assoc() is not 0) in the following execution. However,
this new value of 'val' is not checked before it is used to assigned to
asoc->user_frag. That means it is possible that the new value of 'val'
could be out of the expected range. This can cause security issues
such as buffer overflows, e.g., the new value of 'val' is used as an index
to access a buffer.
This patch inserts a check for the new value of 'val' to see if it is in
the expected range. If it is not, an error code -EINVAL will be returned.
Signed-off-by: Wenwen Wang <wang6495@umn.edu>
---
net/sctp/socket.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 80835ac..03e1cc3 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3212,6 +3212,7 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
struct sctp_af *af = sp->pf->af;
struct sctp_assoc_value params;
struct sctp_association *asoc;
+ int min_len, max_len;
int val;
if (optlen == sizeof(int)) {
@@ -3231,19 +3232,15 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
return -EINVAL;
}
- if (val) {
- int min_len, max_len;
+ min_len = SCTP_DEFAULT_MINSEGMENT - af->net_header_len;
+ min_len -= af->ip_options_len(sk);
+ min_len -= sizeof(struct sctphdr) +
+ sizeof(struct sctp_data_chunk);
- min_len = SCTP_DEFAULT_MINSEGMENT - af->net_header_len;
- min_len -= af->ip_options_len(sk);
- min_len -= sizeof(struct sctphdr) +
- sizeof(struct sctp_data_chunk);
+ max_len = SCTP_MAX_CHUNK_LEN - sizeof(struct sctp_data_chunk);
- max_len = SCTP_MAX_CHUNK_LEN - sizeof(struct sctp_data_chunk);
-
- if (val < min_len || val > max_len)
- return -EINVAL;
- }
+ if (val && (val < min_len || val > max_len))
+ return -EINVAL;
asoc = sctp_id2assoc(sk, params.assoc_id);
if (asoc) {
@@ -3253,6 +3250,9 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
val -= sizeof(struct sctphdr) +
sctp_datachk_len(&asoc->stream);
}
+ /* Check the new val to make sure it is in the range. */
+ if (val < min_len || val > max_len)
+ return -EINVAL;
asoc->user_frag = val;
asoc->frag_point = sctp_frag_point(asoc, asoc->pathmtu);
} else {
--
2.7.4
^ permalink raw reply related
* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Tiwei Bie @ 2018-05-03 1:11 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, virtualization, linux-kernel, netdev, wexu, jfreimann
In-Reply-To: <20180502184015-mutt-send-email-mst@kernel.org>
On Wed, May 02, 2018 at 06:42:57PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 02, 2018 at 11:12:55PM +0800, Tiwei Bie wrote:
> > On Wed, May 02, 2018 at 04:51:01PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 02, 2018 at 03:28:19PM +0800, Tiwei Bie wrote:
> > > > On Wed, May 02, 2018 at 10:51:06AM +0800, Jason Wang wrote:
> > > > > On 2018年04月25日 13:15, Tiwei Bie wrote:
> > > > > > This commit introduces the event idx support in packed
> > > > > > ring. This feature is temporarily disabled, because the
> > > > > > implementation in this patch may not work as expected,
> > > > > > and some further discussions on the implementation are
> > > > > > needed, e.g. do we have to check the wrap counter when
> > > > > > checking whether a kick is needed?
> > > > > >
> > > > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > > > ---
> > > > > > drivers/virtio/virtio_ring.c | 53 ++++++++++++++++++++++++++++++++++++++++----
> > > > > > 1 file changed, 49 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > > > index 0181e93897be..b1039c2985b9 100644
> > > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > > @@ -986,7 +986,7 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
> > > > > > static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
> > > > > > {
> > > > > > struct vring_virtqueue *vq = to_vvq(_vq);
> > > > > > - u16 flags;
> > > > > > + u16 new, old, off_wrap, flags;
> > > > > > bool needs_kick;
> > > > > > u32 snapshot;
> > > > > > @@ -995,7 +995,12 @@ static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
> > > > > > * suppressions. */
> > > > > > virtio_mb(vq->weak_barriers);
> > > > > > + old = vq->next_avail_idx - vq->num_added;
> > > > > > + new = vq->next_avail_idx;
> > > > > > + vq->num_added = 0;
> > > > > > +
> > > > > > snapshot = *(u32 *)vq->vring_packed.device;
> > > > > > + off_wrap = virtio16_to_cpu(_vq->vdev, snapshot & 0xffff);
> > > > > > flags = cpu_to_virtio16(_vq->vdev, snapshot >> 16) & 0x3;
> > > > > > #ifdef DEBUG
> > > > > > @@ -1006,7 +1011,10 @@ static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
> > > > > > vq->last_add_time_valid = false;
> > > > > > #endif
> > > > > > - needs_kick = (flags != VRING_EVENT_F_DISABLE);
> > > > > > + if (flags == VRING_EVENT_F_DESC)
> > > > > > + needs_kick = vring_need_event(off_wrap & ~(1<<15), new, old);
> > > > >
> > > > > I wonder whether or not the math is correct. Both new and event are in the
> > > > > unit of descriptor ring size, but old looks not.
> > > >
> > > > What vring_need_event() cares is the distance between
> > > > `new` and `old`, i.e. vq->num_added. So I think there
> > > > is nothing wrong with `old`. But the calculation of the
> > > > distance between `new` and `event_idx` isn't right when
> > > > `new` wraps. How do you think about the below code:
> > > >
> > > > wrap_counter = off_wrap >> 15;
> > > > event_idx = off_wrap & ~(1<<15);
> > > > if (wrap_counter != vq->wrap_counter)
> > > > event_idx -= vq->vring_packed.num;
> > > >
> > > > needs_kick = vring_need_event(event_idx, new, old);
> > >
> > > I suspect this hack won't work for non power of 2 ring.
> >
> > Above code doesn't require the ring size to be a power of 2.
> >
> > For (__u16)(new_idx - old), what we want to get is vq->num_added.
> >
> > old = vq->next_avail_idx - vq->num_added;
> > new = vq->next_avail_idx;
> >
> > When vq->next_avail_idx >= vq->num_added, it's obvious that,
> > (__u16)(new_idx - old) is vq->num_added.
> >
> > And when vq->next_avail_idx < vq->num_added, new will be smaller
> > than old (old will be a big unsigned number), but (__u16)(new_idx
> > - old) is still vq->num_added.
> >
> > For (__u16)(new_idx - event_idx - 1), when new wraps and event_idx
> > doesn't wrap, the most straightforward way to calculate it is:
> > (new + vq->vring_packed.num) - event_idx - 1.
>
> So how about we use the straightforward way then?
You mean we do new += vq->vring_packed.num instead
of event_idx -= vq->vring_packed.num before calling
vring_need_event()?
The problem is that, the second param (new_idx) of
vring_need_event() will be used for:
(__u16)(new_idx - event_idx - 1)
(__u16)(new_idx - old)
So if we change new, we will need to change old too.
And that would be an ugly hack..
Best regards,
Tiwei Bie
>
> > But we can also calculate it in this way:
> >
> > event_idx -= vq->vring_packed.num;
> > (event_idx will be a big unsigned number)
> >
> > Then (__u16)(new_idx - event_idx - 1) will be the value we want.
> >
> > Best regards,
> > Tiwei Bie
>
>
> > >
> > >
> > > > Best regards,
> > > > Tiwei Bie
> > > >
> > > >
> > > > >
> > > > > Thanks
> > > > >
> > > > > > + else
> > > > > > + needs_kick = (flags != VRING_EVENT_F_DISABLE);
> > > > > > END_USE(vq);
> > > > > > return needs_kick;
> > > > > > }
> > > > > > @@ -1116,6 +1124,15 @@ static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
> > > > > > if (vq->last_used_idx >= vq->vring_packed.num)
> > > > > > vq->last_used_idx -= vq->vring_packed.num;
> > > > > > + /* If we expect an interrupt for the next entry, tell host
> > > > > > + * by writing event index and flush out the write before
> > > > > > + * the read in the next get_buf call. */
> > > > > > + if (vq->event_flags_shadow == VRING_EVENT_F_DESC)
> > > > > > + virtio_store_mb(vq->weak_barriers,
> > > > > > + &vq->vring_packed.driver->off_wrap,
> > > > > > + cpu_to_virtio16(_vq->vdev, vq->last_used_idx |
> > > > > > + (vq->wrap_counter << 15)));
> > > > > > +
> > > > > > #ifdef DEBUG
> > > > > > vq->last_add_time_valid = false;
> > > > > > #endif
> > > > > > @@ -1143,10 +1160,17 @@ static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq)
> > > > > > /* We optimistically turn back on interrupts, then check if there was
> > > > > > * more to do. */
> > > > > > + /* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
> > > > > > + * either clear the flags bit or point the event index at the next
> > > > > > + * entry. Always update the event index to keep code simple. */
> > > > > > +
> > > > > > + vq->vring_packed.driver->off_wrap = cpu_to_virtio16(_vq->vdev,
> > > > > > + vq->last_used_idx | (vq->wrap_counter << 15));
> > > > > > if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
> > > > > > virtio_wmb(vq->weak_barriers);
> > > > > > - vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
> > > > > > + vq->event_flags_shadow = vq->event ? VRING_EVENT_F_DESC :
> > > > > > + VRING_EVENT_F_ENABLE;
> > > > > > vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
> > > > > > vq->event_flags_shadow);
> > > > > > }
> > > > > > @@ -1172,15 +1196,34 @@ static bool virtqueue_poll_packed(struct virtqueue *_vq, unsigned last_used_idx)
> > > > > > static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq)
> > > > > > {
> > > > > > struct vring_virtqueue *vq = to_vvq(_vq);
> > > > > > + u16 bufs, used_idx, wrap_counter;
> > > > > > START_USE(vq);
> > > > > > /* We optimistically turn back on interrupts, then check if there was
> > > > > > * more to do. */
> > > > > > + /* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
> > > > > > + * either clear the flags bit or point the event index at the next
> > > > > > + * entry. Always update the event index to keep code simple. */
> > > > > > +
> > > > > > + /* TODO: tune this threshold */
> > > > > > + bufs = (u16)(vq->next_avail_idx - vq->last_used_idx) * 3 / 4;
> > > > > > +
> > > > > > + used_idx = vq->last_used_idx + bufs;
> > > > > > + wrap_counter = vq->wrap_counter;
> > > > > > +
> > > > > > + if (used_idx >= vq->vring_packed.num) {
> > > > > > + used_idx -= vq->vring_packed.num;
> > > > > > + wrap_counter ^= 1;
> > > > > > + }
> > > > > > +
> > > > > > + vq->vring_packed.driver->off_wrap = cpu_to_virtio16(_vq->vdev,
> > > > > > + used_idx | (wrap_counter << 15));
> > > > > > if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
> > > > > > virtio_wmb(vq->weak_barriers);
> > > > > > - vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
> > > > > > + vq->event_flags_shadow = vq->event ? VRING_EVENT_F_DESC :
> > > > > > + VRING_EVENT_F_ENABLE;
> > > > > > vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
> > > > > > vq->event_flags_shadow);
> > > > > > }
> > > > > > @@ -1822,8 +1865,10 @@ void vring_transport_features(struct virtio_device *vdev)
> > > > > > switch (i) {
> > > > > > case VIRTIO_RING_F_INDIRECT_DESC:
> > > > > > break;
> > > > > > +#if 0
> > > > > > case VIRTIO_RING_F_EVENT_IDX:
> > > > > > break;
> > > > > > +#endif
> > > > > > case VIRTIO_F_VERSION_1:
> > > > > > break;
> > > > > > case VIRTIO_F_IOMMU_PLATFORM:
> > > > >
^ permalink raw reply
* Re: [PATCH] sctp: fix a potential missing-check bug
From: Wenwen Wang @ 2018-05-03 1:07 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Kangjie Lu, Vlad Yasevich, Neil Horman, David S. Miller,
open list:SCTP PROTOCOL, open list:NETWORKING [GENERAL],
open list, Wenwen Wang
In-Reply-To: <20180502232352.GJ5105@localhost.localdomain>
Hi Marcelo,
I guess I worked on an old version of the kernel. I will re-submit the
patch. Sorry :(
Wenwen
On Wed, May 2, 2018 at 6:23 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> Hi Wenwen,
>
> On Wed, May 02, 2018 at 05:12:45PM -0500, Wenwen Wang wrote:
>> In sctp_setsockopt_maxseg(), the integer 'val' is compared against min_len
>> and max_len to check whether it is in the appropriate range. If it is not,
>> an error code -EINVAL will be returned. This is enforced by a security
>> check. But, this check is only executed when 'val' is not 0. In fact, if
>
> Which makes sense, no? Especially if considering that 0 should be an
> allowed value as it turns off the user limit.
>
>> 'val' is 0, it will be assigned with a new value (if the return value of
>> the function sctp_id2assoc() is not 0) in the following execution. However,
>> this new value of 'val' is not checked before it is used to assigned to
>
> Which 'new value'? val is not set to something new during the
> function. It always contains the user supplied value.
>
>> asoc->user_frag. That means it is possible that the new value of 'val'
>> could be out of the expected range. This can cause security issues
>> such as buffer overflows, e.g., the new value of 'val' is used as an index
>> to access a buffer.
>>
>> This patch inserts a check for the new value of 'val' to see if it is in
>> the expected range. If it is not, an error code -EINVAL will be returned.
>>
>> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
>> ---
>> net/sctp/socket.c | 21 ++++++++++-----------
>> 1 file changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>> index 80835ac..2beb601 100644
>> --- a/net/sctp/socket.c
>> +++ b/net/sctp/socket.c
>> @@ -3212,6 +3212,7 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
>> struct sctp_af *af = sp->pf->af;
>> struct sctp_assoc_value params;
>> struct sctp_association *asoc;
>> + int min_len, max_len;
>> int val;
>>
>> if (optlen == sizeof(int)) {
>> @@ -3231,19 +3232,15 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
>> return -EINVAL;
>> }
>>
>> - if (val) {
>> - int min_len, max_len;
>> + min_len = SCTP_DEFAULT_MINSEGMENT - af->net_header_len;
>> + min_len -= af->ip_options_len(sk);
>> + min_len -= sizeof(struct sctphdr) +
>> + sizeof(struct sctp_data_chunk);
>
> On which tree did you base your patch on? Your patch lacks a tag so it
> defaults to net-next, and I reworked this section on current net-next
> and these MTU calculcations are now handled by sctp_mtu_payload().
>
> But even for net tree, I don't understand which issue you're fixing
> here. Actually it seems to me that both codes seems to do the same
> thing.
>
>>
>> - min_len = SCTP_DEFAULT_MINSEGMENT - af->net_header_len;
>> - min_len -= af->ip_options_len(sk);
>> - min_len -= sizeof(struct sctphdr) +
>> - sizeof(struct sctp_data_chunk);
>> + max_len = SCTP_MAX_CHUNK_LEN - sizeof(struct sctp_data_chunk);
>>
>> - max_len = SCTP_MAX_CHUNK_LEN - sizeof(struct sctp_data_chunk);
>> -
>> - if (val < min_len || val > max_len)
>> - return -EINVAL;
>> - }
>> + if (val && (val < min_len || val > max_len))
>> + return -EINVAL;
>>
>> asoc = sctp_id2assoc(sk, params.assoc_id);
>> if (asoc) {
>> @@ -3253,6 +3250,8 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
>> val -= sizeof(struct sctphdr) +
>> sctp_datachk_len(&asoc->stream);
>> }
>> + if (val < min_len || val > max_len)
>> + return -EINVAL;
>> asoc->user_frag = val;
>> asoc->frag_point = sctp_frag_point(asoc, asoc->pathmtu);
>> } else {
>> --
>> 2.7.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
^ permalink raw reply
* [PATCH bpf-next 07/12] bpf, sparc64: remove ld_abs/ld_ind
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann, David S . Miller
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
Since LD_ABS/LD_IND instructions are now removed from the core and
reimplemented through a combination of inlined BPF instructions and
a slow-path helper, we can get rid of the complexity from sparc64 JIT.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
arch/sparc/net/Makefile | 5 +-
arch/sparc/net/bpf_jit_64.h | 29 -------
arch/sparc/net/bpf_jit_asm_64.S | 162 ---------------------------------------
arch/sparc/net/bpf_jit_comp_64.c | 79 +------------------
4 files changed, 6 insertions(+), 269 deletions(-)
delete mode 100644 arch/sparc/net/bpf_jit_asm_64.S
diff --git a/arch/sparc/net/Makefile b/arch/sparc/net/Makefile
index 76fa8e9..d32aac3 100644
--- a/arch/sparc/net/Makefile
+++ b/arch/sparc/net/Makefile
@@ -1,4 +1,7 @@
#
# Arch-specific network modules
#
-obj-$(CONFIG_BPF_JIT) += bpf_jit_asm_$(BITS).o bpf_jit_comp_$(BITS).o
+obj-$(CONFIG_BPF_JIT) += bpf_jit_comp_$(BITS).o
+ifeq ($(BITS),32)
+obj-$(CONFIG_BPF_JIT) += bpf_jit_asm_32.o
+endif
diff --git a/arch/sparc/net/bpf_jit_64.h b/arch/sparc/net/bpf_jit_64.h
index 428f7fd..fbc836f 100644
--- a/arch/sparc/net/bpf_jit_64.h
+++ b/arch/sparc/net/bpf_jit_64.h
@@ -33,35 +33,6 @@
#define I5 0x1d
#define FP 0x1e
#define I7 0x1f
-
-#define r_SKB L0
-#define r_HEADLEN L4
-#define r_SKB_DATA L5
-#define r_TMP G1
-#define r_TMP2 G3
-
-/* assembly code in arch/sparc/net/bpf_jit_asm_64.S */
-extern u32 bpf_jit_load_word[];
-extern u32 bpf_jit_load_half[];
-extern u32 bpf_jit_load_byte[];
-extern u32 bpf_jit_load_byte_msh[];
-extern u32 bpf_jit_load_word_positive_offset[];
-extern u32 bpf_jit_load_half_positive_offset[];
-extern u32 bpf_jit_load_byte_positive_offset[];
-extern u32 bpf_jit_load_byte_msh_positive_offset[];
-extern u32 bpf_jit_load_word_negative_offset[];
-extern u32 bpf_jit_load_half_negative_offset[];
-extern u32 bpf_jit_load_byte_negative_offset[];
-extern u32 bpf_jit_load_byte_msh_negative_offset[];
-
-#else
-#define r_RESULT %o0
-#define r_SKB %o0
-#define r_OFF %o1
-#define r_HEADLEN %l4
-#define r_SKB_DATA %l5
-#define r_TMP %g1
-#define r_TMP2 %g3
#endif
#endif /* _BPF_JIT_H */
diff --git a/arch/sparc/net/bpf_jit_asm_64.S b/arch/sparc/net/bpf_jit_asm_64.S
deleted file mode 100644
index 7177867..0000000
--- a/arch/sparc/net/bpf_jit_asm_64.S
+++ /dev/null
@@ -1,162 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#include <asm/ptrace.h>
-
-#include "bpf_jit_64.h"
-
-#define SAVE_SZ 176
-#define SCRATCH_OFF STACK_BIAS + 128
-#define BE_PTR(label) be,pn %xcc, label
-#define SIGN_EXTEND(reg) sra reg, 0, reg
-
-#define SKF_MAX_NEG_OFF (-0x200000) /* SKF_LL_OFF from filter.h */
-
- .text
- .globl bpf_jit_load_word
-bpf_jit_load_word:
- cmp r_OFF, 0
- bl bpf_slow_path_word_neg
- nop
- .globl bpf_jit_load_word_positive_offset
-bpf_jit_load_word_positive_offset:
- sub r_HEADLEN, r_OFF, r_TMP
- cmp r_TMP, 3
- ble bpf_slow_path_word
- add r_SKB_DATA, r_OFF, r_TMP
- andcc r_TMP, 3, %g0
- bne load_word_unaligned
- nop
- retl
- ld [r_TMP], r_RESULT
-load_word_unaligned:
- ldub [r_TMP + 0x0], r_OFF
- ldub [r_TMP + 0x1], r_TMP2
- sll r_OFF, 8, r_OFF
- or r_OFF, r_TMP2, r_OFF
- ldub [r_TMP + 0x2], r_TMP2
- sll r_OFF, 8, r_OFF
- or r_OFF, r_TMP2, r_OFF
- ldub [r_TMP + 0x3], r_TMP2
- sll r_OFF, 8, r_OFF
- retl
- or r_OFF, r_TMP2, r_RESULT
-
- .globl bpf_jit_load_half
-bpf_jit_load_half:
- cmp r_OFF, 0
- bl bpf_slow_path_half_neg
- nop
- .globl bpf_jit_load_half_positive_offset
-bpf_jit_load_half_positive_offset:
- sub r_HEADLEN, r_OFF, r_TMP
- cmp r_TMP, 1
- ble bpf_slow_path_half
- add r_SKB_DATA, r_OFF, r_TMP
- andcc r_TMP, 1, %g0
- bne load_half_unaligned
- nop
- retl
- lduh [r_TMP], r_RESULT
-load_half_unaligned:
- ldub [r_TMP + 0x0], r_OFF
- ldub [r_TMP + 0x1], r_TMP2
- sll r_OFF, 8, r_OFF
- retl
- or r_OFF, r_TMP2, r_RESULT
-
- .globl bpf_jit_load_byte
-bpf_jit_load_byte:
- cmp r_OFF, 0
- bl bpf_slow_path_byte_neg
- nop
- .globl bpf_jit_load_byte_positive_offset
-bpf_jit_load_byte_positive_offset:
- cmp r_OFF, r_HEADLEN
- bge bpf_slow_path_byte
- nop
- retl
- ldub [r_SKB_DATA + r_OFF], r_RESULT
-
-#define bpf_slow_path_common(LEN) \
- save %sp, -SAVE_SZ, %sp; \
- mov %i0, %o0; \
- mov %i1, %o1; \
- add %fp, SCRATCH_OFF, %o2; \
- call skb_copy_bits; \
- mov (LEN), %o3; \
- cmp %o0, 0; \
- restore;
-
-bpf_slow_path_word:
- bpf_slow_path_common(4)
- bl bpf_error
- ld [%sp + SCRATCH_OFF], r_RESULT
- retl
- nop
-bpf_slow_path_half:
- bpf_slow_path_common(2)
- bl bpf_error
- lduh [%sp + SCRATCH_OFF], r_RESULT
- retl
- nop
-bpf_slow_path_byte:
- bpf_slow_path_common(1)
- bl bpf_error
- ldub [%sp + SCRATCH_OFF], r_RESULT
- retl
- nop
-
-#define bpf_negative_common(LEN) \
- save %sp, -SAVE_SZ, %sp; \
- mov %i0, %o0; \
- mov %i1, %o1; \
- SIGN_EXTEND(%o1); \
- call bpf_internal_load_pointer_neg_helper; \
- mov (LEN), %o2; \
- mov %o0, r_TMP; \
- cmp %o0, 0; \
- BE_PTR(bpf_error); \
- restore;
-
-bpf_slow_path_word_neg:
- sethi %hi(SKF_MAX_NEG_OFF), r_TMP
- cmp r_OFF, r_TMP
- bl bpf_error
- nop
- .globl bpf_jit_load_word_negative_offset
-bpf_jit_load_word_negative_offset:
- bpf_negative_common(4)
- andcc r_TMP, 3, %g0
- bne load_word_unaligned
- nop
- retl
- ld [r_TMP], r_RESULT
-
-bpf_slow_path_half_neg:
- sethi %hi(SKF_MAX_NEG_OFF), r_TMP
- cmp r_OFF, r_TMP
- bl bpf_error
- nop
- .globl bpf_jit_load_half_negative_offset
-bpf_jit_load_half_negative_offset:
- bpf_negative_common(2)
- andcc r_TMP, 1, %g0
- bne load_half_unaligned
- nop
- retl
- lduh [r_TMP], r_RESULT
-
-bpf_slow_path_byte_neg:
- sethi %hi(SKF_MAX_NEG_OFF), r_TMP
- cmp r_OFF, r_TMP
- bl bpf_error
- nop
- .globl bpf_jit_load_byte_negative_offset
-bpf_jit_load_byte_negative_offset:
- bpf_negative_common(1)
- retl
- ldub [r_TMP], r_RESULT
-
-bpf_error:
- /* Make the JIT program itself return zero. */
- ret
- restore %g0, %g0, %o0
diff --git a/arch/sparc/net/bpf_jit_comp_64.c b/arch/sparc/net/bpf_jit_comp_64.c
index 48a2586..9f5918e 100644
--- a/arch/sparc/net/bpf_jit_comp_64.c
+++ b/arch/sparc/net/bpf_jit_comp_64.c
@@ -48,10 +48,6 @@ static void bpf_flush_icache(void *start_, void *end_)
}
}
-#define SEEN_DATAREF 1 /* might call external helpers */
-#define SEEN_XREG 2 /* ebx is used */
-#define SEEN_MEM 4 /* use mem[] for temporary storage */
-
#define S13(X) ((X) & 0x1fff)
#define S5(X) ((X) & 0x1f)
#define IMMED 0x00002000
@@ -198,7 +194,6 @@ struct jit_ctx {
bool tmp_1_used;
bool tmp_2_used;
bool tmp_3_used;
- bool saw_ld_abs_ind;
bool saw_frame_pointer;
bool saw_call;
bool saw_tail_call;
@@ -207,9 +202,7 @@ struct jit_ctx {
#define TMP_REG_1 (MAX_BPF_JIT_REG + 0)
#define TMP_REG_2 (MAX_BPF_JIT_REG + 1)
-#define SKB_HLEN_REG (MAX_BPF_JIT_REG + 2)
-#define SKB_DATA_REG (MAX_BPF_JIT_REG + 3)
-#define TMP_REG_3 (MAX_BPF_JIT_REG + 4)
+#define TMP_REG_3 (MAX_BPF_JIT_REG + 2)
/* Map BPF registers to SPARC registers */
static const int bpf2sparc[] = {
@@ -238,9 +231,6 @@ static const int bpf2sparc[] = {
[TMP_REG_1] = G1,
[TMP_REG_2] = G2,
[TMP_REG_3] = G3,
-
- [SKB_HLEN_REG] = L4,
- [SKB_DATA_REG] = L5,
};
static void emit(const u32 insn, struct jit_ctx *ctx)
@@ -800,25 +790,6 @@ static int emit_compare_and_branch(const u8 code, const u8 dst, u8 src,
return 0;
}
-static void load_skb_regs(struct jit_ctx *ctx, u8 r_skb)
-{
- const u8 r_headlen = bpf2sparc[SKB_HLEN_REG];
- const u8 r_data = bpf2sparc[SKB_DATA_REG];
- const u8 r_tmp = bpf2sparc[TMP_REG_1];
- unsigned int off;
-
- off = offsetof(struct sk_buff, len);
- emit(LD32I | RS1(r_skb) | S13(off) | RD(r_headlen), ctx);
-
- off = offsetof(struct sk_buff, data_len);
- emit(LD32I | RS1(r_skb) | S13(off) | RD(r_tmp), ctx);
-
- emit(SUB | RS1(r_headlen) | RS2(r_tmp) | RD(r_headlen), ctx);
-
- off = offsetof(struct sk_buff, data);
- emit(LDPTRI | RS1(r_skb) | S13(off) | RD(r_data), ctx);
-}
-
/* Just skip the save instruction and the ctx register move. */
#define BPF_TAILCALL_PROLOGUE_SKIP 16
#define BPF_TAILCALL_CNT_SP_OFF (STACK_BIAS + 128)
@@ -857,9 +828,6 @@ static void build_prologue(struct jit_ctx *ctx)
emit_reg_move(I0, O0, ctx);
/* If you add anything here, adjust BPF_TAILCALL_PROLOGUE_SKIP above. */
-
- if (ctx->saw_ld_abs_ind)
- load_skb_regs(ctx, bpf2sparc[BPF_REG_1]);
}
static void build_epilogue(struct jit_ctx *ctx)
@@ -1225,16 +1193,11 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
u8 *func = ((u8 *)__bpf_call_base) + imm;
ctx->saw_call = true;
- if (ctx->saw_ld_abs_ind && bpf_helper_changes_pkt_data(func))
- emit_reg_move(bpf2sparc[BPF_REG_1], L7, ctx);
emit_call((u32 *)func, ctx);
emit_nop(ctx);
emit_reg_move(O0, bpf2sparc[BPF_REG_0], ctx);
-
- if (ctx->saw_ld_abs_ind && bpf_helper_changes_pkt_data(func))
- load_skb_regs(ctx, L7);
break;
}
@@ -1412,43 +1375,6 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
emit_nop(ctx);
break;
}
-#define CHOOSE_LOAD_FUNC(K, func) \
- ((int)K < 0 ? ((int)K >= SKF_LL_OFF ? func##_negative_offset : func) : func##_positive_offset)
-
- /* R0 = ntohx(*(size *)(((struct sk_buff *)R6)->data + imm)) */
- case BPF_LD | BPF_ABS | BPF_W:
- func = CHOOSE_LOAD_FUNC(imm, bpf_jit_load_word);
- goto common_load;
- case BPF_LD | BPF_ABS | BPF_H:
- func = CHOOSE_LOAD_FUNC(imm, bpf_jit_load_half);
- goto common_load;
- case BPF_LD | BPF_ABS | BPF_B:
- func = CHOOSE_LOAD_FUNC(imm, bpf_jit_load_byte);
- goto common_load;
- /* R0 = ntohx(*(size *)(((struct sk_buff *)R6)->data + src + imm)) */
- case BPF_LD | BPF_IND | BPF_W:
- func = bpf_jit_load_word;
- goto common_load;
- case BPF_LD | BPF_IND | BPF_H:
- func = bpf_jit_load_half;
- goto common_load;
-
- case BPF_LD | BPF_IND | BPF_B:
- func = bpf_jit_load_byte;
- common_load:
- ctx->saw_ld_abs_ind = true;
-
- emit_reg_move(bpf2sparc[BPF_REG_6], O0, ctx);
- emit_loadimm(imm, O1, ctx);
-
- if (BPF_MODE(code) == BPF_IND)
- emit_alu(ADD, src, O1, ctx);
-
- emit_call(func, ctx);
- emit_alu_K(SRA, O1, 0, ctx);
-
- emit_reg_move(O0, bpf2sparc[BPF_REG_0], ctx);
- break;
default:
pr_err_once("unknown opcode %02x\n", code);
@@ -1583,12 +1509,11 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
build_epilogue(&ctx);
if (bpf_jit_enable > 1)
- pr_info("Pass %d: shrink = %d, seen = [%c%c%c%c%c%c%c]\n", pass,
+ pr_info("Pass %d: shrink = %d, seen = [%c%c%c%c%c%c]\n", pass,
image_size - (ctx.idx * 4),
ctx.tmp_1_used ? '1' : ' ',
ctx.tmp_2_used ? '2' : ' ',
ctx.tmp_3_used ? '3' : ' ',
- ctx.saw_ld_abs_ind ? 'L' : ' ',
ctx.saw_frame_pointer ? 'F' : ' ',
ctx.saw_call ? 'C' : ' ',
ctx.saw_tail_call ? 'T' : ' ');
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 11/12] bpf, s390x: remove ld_abs/ld_ind
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann, Michael Holzheu
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
Since LD_ABS/LD_IND instructions are now removed from the core and
reimplemented through a combination of inlined BPF instructions and
a slow-path helper, we can get rid of the complexity from s390x JIT.
Tested on s390x instance on LinuxONE.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Michael Holzheu <holzheu@linux.vnet.ibm.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
arch/s390/net/Makefile | 2 +-
arch/s390/net/bpf_jit.S | 116 ---------------------------------------
arch/s390/net/bpf_jit.h | 20 +------
arch/s390/net/bpf_jit_comp.c | 127 ++++---------------------------------------
4 files changed, 13 insertions(+), 252 deletions(-)
delete mode 100644 arch/s390/net/bpf_jit.S
diff --git a/arch/s390/net/Makefile b/arch/s390/net/Makefile
index e0d5f24..d4663b4 100644
--- a/arch/s390/net/Makefile
+++ b/arch/s390/net/Makefile
@@ -2,4 +2,4 @@
#
# Arch-specific network modules
#
-obj-$(CONFIG_BPF_JIT) += bpf_jit.o bpf_jit_comp.o
+obj-$(CONFIG_BPF_JIT) += bpf_jit_comp.o
diff --git a/arch/s390/net/bpf_jit.S b/arch/s390/net/bpf_jit.S
deleted file mode 100644
index 25bb464..0000000
--- a/arch/s390/net/bpf_jit.S
+++ /dev/null
@@ -1,116 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * BPF Jit compiler for s390, help functions.
- *
- * Copyright IBM Corp. 2012,2015
- *
- * Author(s): Martin Schwidefsky <schwidefsky@de.ibm.com>
- * Michael Holzheu <holzheu@linux.vnet.ibm.com>
- */
-
-#include <linux/linkage.h>
-#include "bpf_jit.h"
-
-/*
- * Calling convention:
- * registers %r7-%r10, %r11,%r13, and %r15 are call saved
- *
- * Input (64 bit):
- * %r3 (%b2) = offset into skb data
- * %r6 (%b5) = return address
- * %r7 (%b6) = skb pointer
- * %r12 = skb data pointer
- *
- * Output:
- * %r14= %b0 = return value (read skb value)
- *
- * Work registers: %r2,%r4,%r5,%r14
- *
- * skb_copy_bits takes 4 parameters:
- * %r2 = skb pointer
- * %r3 = offset into skb data
- * %r4 = pointer to temp buffer
- * %r5 = length to copy
- * Return value in %r2: 0 = ok
- *
- * bpf_internal_load_pointer_neg_helper takes 3 parameters:
- * %r2 = skb pointer
- * %r3 = offset into data
- * %r4 = length to copy
- * Return value in %r2: Pointer to data
- */
-
-#define SKF_MAX_NEG_OFF -0x200000 /* SKF_LL_OFF from filter.h */
-
-/*
- * Load SIZE bytes from SKB
- */
-#define sk_load_common(NAME, SIZE, LOAD) \
-ENTRY(sk_load_##NAME); \
- ltgr %r3,%r3; /* Is offset negative? */ \
- jl sk_load_##NAME##_slow_neg; \
-ENTRY(sk_load_##NAME##_pos); \
- aghi %r3,SIZE; /* Offset + SIZE */ \
- clg %r3,STK_OFF_HLEN(%r15); /* Offset + SIZE > hlen? */ \
- jh sk_load_##NAME##_slow; \
- LOAD %r14,-SIZE(%r3,%r12); /* Get data from skb */ \
- b OFF_OK(%r6); /* Return */ \
- \
-sk_load_##NAME##_slow:; \
- lgr %r2,%r7; /* Arg1 = skb pointer */ \
- aghi %r3,-SIZE; /* Arg2 = offset */ \
- la %r4,STK_OFF_TMP(%r15); /* Arg3 = temp bufffer */ \
- lghi %r5,SIZE; /* Arg4 = size */ \
- brasl %r14,skb_copy_bits; /* Get data from skb */ \
- LOAD %r14,STK_OFF_TMP(%r15); /* Load from temp bufffer */ \
- ltgr %r2,%r2; /* Set cc to (%r2 != 0) */ \
- br %r6; /* Return */
-
-sk_load_common(word, 4, llgf) /* r14 = *(u32 *) (skb->data+offset) */
-sk_load_common(half, 2, llgh) /* r14 = *(u16 *) (skb->data+offset) */
-
-/*
- * Load 1 byte from SKB (optimized version)
- */
- /* r14 = *(u8 *) (skb->data+offset) */
-ENTRY(sk_load_byte)
- ltgr %r3,%r3 # Is offset negative?
- jl sk_load_byte_slow_neg
-ENTRY(sk_load_byte_pos)
- clg %r3,STK_OFF_HLEN(%r15) # Offset >= hlen?
- jnl sk_load_byte_slow
- llgc %r14,0(%r3,%r12) # Get byte from skb
- b OFF_OK(%r6) # Return OK
-
-sk_load_byte_slow:
- lgr %r2,%r7 # Arg1 = skb pointer
- # Arg2 = offset
- la %r4,STK_OFF_TMP(%r15) # Arg3 = pointer to temp buffer
- lghi %r5,1 # Arg4 = size (1 byte)
- brasl %r14,skb_copy_bits # Get data from skb
- llgc %r14,STK_OFF_TMP(%r15) # Load result from temp buffer
- ltgr %r2,%r2 # Set cc to (%r2 != 0)
- br %r6 # Return cc
-
-#define sk_negative_common(NAME, SIZE, LOAD) \
-sk_load_##NAME##_slow_neg:; \
- cgfi %r3,SKF_MAX_NEG_OFF; \
- jl bpf_error; \
- lgr %r2,%r7; /* Arg1 = skb pointer */ \
- /* Arg2 = offset */ \
- lghi %r4,SIZE; /* Arg3 = size */ \
- brasl %r14,bpf_internal_load_pointer_neg_helper; \
- ltgr %r2,%r2; \
- jz bpf_error; \
- LOAD %r14,0(%r2); /* Get data from pointer */ \
- xr %r3,%r3; /* Set cc to zero */ \
- br %r6; /* Return cc */
-
-sk_negative_common(word, 4, llgf)
-sk_negative_common(half, 2, llgh)
-sk_negative_common(byte, 1, llgc)
-
-bpf_error:
-# force a return 0 from jit handler
- ltgr %r15,%r15 # Set condition code
- br %r6
diff --git a/arch/s390/net/bpf_jit.h b/arch/s390/net/bpf_jit.h
index 5e1e513..7822ea9 100644
--- a/arch/s390/net/bpf_jit.h
+++ b/arch/s390/net/bpf_jit.h
@@ -16,9 +16,6 @@
#include <linux/filter.h>
#include <linux/types.h>
-extern u8 sk_load_word_pos[], sk_load_half_pos[], sk_load_byte_pos[];
-extern u8 sk_load_word[], sk_load_half[], sk_load_byte[];
-
#endif /* __ASSEMBLY__ */
/*
@@ -36,15 +33,6 @@ extern u8 sk_load_word[], sk_load_half[], sk_load_byte[];
* | | |
* | BPF stack | |
* | | |
- * +---------------+ |
- * | 8 byte skbp | |
- * R15+176 -> +---------------+ |
- * | 8 byte hlen | |
- * R15+168 -> +---------------+ |
- * | 4 byte align | |
- * +---------------+ |
- * | 4 byte temp | |
- * | for bpf_jit.S | |
* R15+160 -> +---------------+ |
* | new backchain | |
* R15+152 -> +---------------+ |
@@ -57,17 +45,11 @@ extern u8 sk_load_word[], sk_load_half[], sk_load_byte[];
* The stack size used by the BPF program ("BPF stack" above) is passed
* via "aux->stack_depth".
*/
-#define STK_SPACE_ADD (8 + 8 + 4 + 4 + 160)
+#define STK_SPACE_ADD (160)
#define STK_160_UNUSED (160 - 12 * 8)
#define STK_OFF (STK_SPACE_ADD - STK_160_UNUSED)
-#define STK_OFF_TMP 160 /* Offset of tmp buffer on stack */
-#define STK_OFF_HLEN 168 /* Offset of SKB header length on stack */
-#define STK_OFF_SKBP 176 /* Offset of SKB pointer on stack */
#define STK_OFF_R6 (160 - 11 * 8) /* Offset of r6 on stack */
#define STK_OFF_TCCNT (160 - 12 * 8) /* Offset of tail_call_cnt on stack */
-/* Offset to skip condition code check */
-#define OFF_OK 4
-
#endif /* __ARCH_S390_NET_BPF_JIT_H */
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 78a19c9..b020bea 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -47,23 +47,21 @@ struct bpf_jit {
#define BPF_SIZE_MAX 0xffff /* Max size for program (16 bit branches) */
-#define SEEN_SKB 1 /* skb access */
-#define SEEN_MEM 2 /* use mem[] for temporary storage */
-#define SEEN_RET0 4 /* ret0_ip points to a valid return 0 */
-#define SEEN_LITERAL 8 /* code uses literals */
-#define SEEN_FUNC 16 /* calls C functions */
-#define SEEN_TAIL_CALL 32 /* code uses tail calls */
-#define SEEN_REG_AX 64 /* code uses constant blinding */
-#define SEEN_STACK (SEEN_FUNC | SEEN_MEM | SEEN_SKB)
+#define SEEN_MEM (1 << 0) /* use mem[] for temporary storage */
+#define SEEN_RET0 (1 << 1) /* ret0_ip points to a valid return 0 */
+#define SEEN_LITERAL (1 << 2) /* code uses literals */
+#define SEEN_FUNC (1 << 3) /* calls C functions */
+#define SEEN_TAIL_CALL (1 << 4) /* code uses tail calls */
+#define SEEN_REG_AX (1 << 5) /* code uses constant blinding */
+#define SEEN_STACK (SEEN_FUNC | SEEN_MEM)
/*
* s390 registers
*/
#define REG_W0 (MAX_BPF_JIT_REG + 0) /* Work register 1 (even) */
#define REG_W1 (MAX_BPF_JIT_REG + 1) /* Work register 2 (odd) */
-#define REG_SKB_DATA (MAX_BPF_JIT_REG + 2) /* SKB data register */
-#define REG_L (MAX_BPF_JIT_REG + 3) /* Literal pool register */
-#define REG_15 (MAX_BPF_JIT_REG + 4) /* Register 15 */
+#define REG_L (MAX_BPF_JIT_REG + 2) /* Literal pool register */
+#define REG_15 (MAX_BPF_JIT_REG + 3) /* Register 15 */
#define REG_0 REG_W0 /* Register 0 */
#define REG_1 REG_W1 /* Register 1 */
#define REG_2 BPF_REG_1 /* Register 2 */
@@ -88,10 +86,8 @@ static const int reg2hex[] = {
[BPF_REG_9] = 10,
/* BPF stack pointer */
[BPF_REG_FP] = 13,
- /* Register for blinding (shared with REG_SKB_DATA) */
+ /* Register for blinding */
[BPF_REG_AX] = 12,
- /* SKB data pointer */
- [REG_SKB_DATA] = 12,
/* Work registers for s390x backend */
[REG_W0] = 0,
[REG_W1] = 1,
@@ -385,27 +381,6 @@ static void save_restore_regs(struct bpf_jit *jit, int op, u32 stack_depth)
}
/*
- * For SKB access %b1 contains the SKB pointer. For "bpf_jit.S"
- * we store the SKB header length on the stack and the SKB data
- * pointer in REG_SKB_DATA if BPF_REG_AX is not used.
- */
-static void emit_load_skb_data_hlen(struct bpf_jit *jit)
-{
- /* Header length: llgf %w1,<len>(%b1) */
- EMIT6_DISP_LH(0xe3000000, 0x0016, REG_W1, REG_0, BPF_REG_1,
- offsetof(struct sk_buff, len));
- /* s %w1,<data_len>(%b1) */
- EMIT4_DISP(0x5b000000, REG_W1, BPF_REG_1,
- offsetof(struct sk_buff, data_len));
- /* stg %w1,ST_OFF_HLEN(%r0,%r15) */
- EMIT6_DISP_LH(0xe3000000, 0x0024, REG_W1, REG_0, REG_15, STK_OFF_HLEN);
- if (!(jit->seen & SEEN_REG_AX))
- /* lg %skb_data,data_off(%b1) */
- EMIT6_DISP_LH(0xe3000000, 0x0004, REG_SKB_DATA, REG_0,
- BPF_REG_1, offsetof(struct sk_buff, data));
-}
-
-/*
* Emit function prologue
*
* Save registers and create stack frame if necessary.
@@ -445,12 +420,6 @@ static void bpf_jit_prologue(struct bpf_jit *jit, u32 stack_depth)
EMIT6_DISP_LH(0xe3000000, 0x0024, REG_W1, REG_0,
REG_15, 152);
}
- if (jit->seen & SEEN_SKB) {
- emit_load_skb_data_hlen(jit);
- /* stg %b1,ST_OFF_SKBP(%r0,%r15) */
- EMIT6_DISP_LH(0xe3000000, 0x0024, BPF_REG_1, REG_0, REG_15,
- STK_OFF_SKBP);
- }
}
/*
@@ -483,12 +452,12 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
{
struct bpf_insn *insn = &fp->insnsi[i];
int jmp_off, last, insn_count = 1;
- unsigned int func_addr, mask;
u32 dst_reg = insn->dst_reg;
u32 src_reg = insn->src_reg;
u32 *addrs = jit->addrs;
s32 imm = insn->imm;
s16 off = insn->off;
+ unsigned int mask;
if (dst_reg == BPF_REG_AX || src_reg == BPF_REG_AX)
jit->seen |= SEEN_REG_AX;
@@ -970,13 +939,6 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
EMIT2(0x0d00, REG_14, REG_W1);
/* lgr %b0,%r2: load return value into %b0 */
EMIT4(0xb9040000, BPF_REG_0, REG_2);
- if ((jit->seen & SEEN_SKB) &&
- bpf_helper_changes_pkt_data((void *)func)) {
- /* lg %b1,ST_OFF_SKBP(%r15) */
- EMIT6_DISP_LH(0xe3000000, 0x0004, BPF_REG_1, REG_0,
- REG_15, STK_OFF_SKBP);
- emit_load_skb_data_hlen(jit);
- }
break;
}
case BPF_JMP | BPF_TAIL_CALL:
@@ -1176,73 +1138,6 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
jmp_off = addrs[i + off + 1] - (addrs[i + 1] - 4);
EMIT4_PCREL(0xa7040000 | mask << 8, jmp_off);
break;
- /*
- * BPF_LD
- */
- case BPF_LD | BPF_ABS | BPF_B: /* b0 = *(u8 *) (skb->data+imm) */
- case BPF_LD | BPF_IND | BPF_B: /* b0 = *(u8 *) (skb->data+imm+src) */
- if ((BPF_MODE(insn->code) == BPF_ABS) && (imm >= 0))
- func_addr = __pa(sk_load_byte_pos);
- else
- func_addr = __pa(sk_load_byte);
- goto call_fn;
- case BPF_LD | BPF_ABS | BPF_H: /* b0 = *(u16 *) (skb->data+imm) */
- case BPF_LD | BPF_IND | BPF_H: /* b0 = *(u16 *) (skb->data+imm+src) */
- if ((BPF_MODE(insn->code) == BPF_ABS) && (imm >= 0))
- func_addr = __pa(sk_load_half_pos);
- else
- func_addr = __pa(sk_load_half);
- goto call_fn;
- case BPF_LD | BPF_ABS | BPF_W: /* b0 = *(u32 *) (skb->data+imm) */
- case BPF_LD | BPF_IND | BPF_W: /* b0 = *(u32 *) (skb->data+imm+src) */
- if ((BPF_MODE(insn->code) == BPF_ABS) && (imm >= 0))
- func_addr = __pa(sk_load_word_pos);
- else
- func_addr = __pa(sk_load_word);
- goto call_fn;
-call_fn:
- jit->seen |= SEEN_SKB | SEEN_RET0 | SEEN_FUNC;
- REG_SET_SEEN(REG_14); /* Return address of possible func call */
-
- /*
- * Implicit input:
- * BPF_REG_6 (R7) : skb pointer
- * REG_SKB_DATA (R12): skb data pointer (if no BPF_REG_AX)
- *
- * Calculated input:
- * BPF_REG_2 (R3) : offset of byte(s) to fetch in skb
- * BPF_REG_5 (R6) : return address
- *
- * Output:
- * BPF_REG_0 (R14): data read from skb
- *
- * Scratch registers (BPF_REG_1-5)
- */
-
- /* Call function: llilf %w1,func_addr */
- EMIT6_IMM(0xc00f0000, REG_W1, func_addr);
-
- /* Offset: lgfi %b2,imm */
- EMIT6_IMM(0xc0010000, BPF_REG_2, imm);
- if (BPF_MODE(insn->code) == BPF_IND)
- /* agfr %b2,%src (%src is s32 here) */
- EMIT4(0xb9180000, BPF_REG_2, src_reg);
-
- /* Reload REG_SKB_DATA if BPF_REG_AX is used */
- if (jit->seen & SEEN_REG_AX)
- /* lg %skb_data,data_off(%b6) */
- EMIT6_DISP_LH(0xe3000000, 0x0004, REG_SKB_DATA, REG_0,
- BPF_REG_6, offsetof(struct sk_buff, data));
- /* basr %b5,%w1 (%b5 is call saved) */
- EMIT2(0x0d00, BPF_REG_5, REG_W1);
-
- /*
- * Note: For fast access we jump directly after the
- * jnz instruction from bpf_jit.S
- */
- /* jnz <ret0> */
- EMIT4_PCREL(0xa7740000, jit->ret0_ip - jit->prg);
- break;
default: /* too complex, give up */
pr_err("Unknown opcode %02x\n", insn->code);
return -1;
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 12/12] bpf: sync tools bpf.h uapi header
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
Only sync the header from include/uapi/linux/bpf.h.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
tools/include/uapi/linux/bpf.h | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 8daef73..83a95ae 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1801,6 +1801,30 @@ union bpf_attr {
* Return
* a non-negative value equal to or less than size on success, or
* a negative error in case of failure.
+ *
+ * int skb_load_bytes_relative(const struct sk_buff *skb, u32 offset, void *to, u32 len, u32 start_header)
+ * Description
+ * This helper is similar to **bpf_skb_load_bytes**\ () in that
+ * it provides an easy way to load *len* bytes from *offset*
+ * from the packet associated to *skb*, into the buffer pointed
+ * by *to*. The difference to **bpf_skb_load_bytes**\ () is that
+ * a fifth argument *start_header* exists in order to select a
+ * base offset to start from. *start_header* can be one of:
+ *
+ * **BPF_HDR_START_MAC**
+ * Base offset to load data from is *skb*'s mac header.
+ * **BPF_HDR_START_NET**
+ * Base offset to load data from is *skb*'s network header.
+ *
+ * In general, "direct packet access" is the preferred method to
+ * access packet data, however, this helper is in particular useful
+ * in socket filters where *skb*\ **->data** does not always point
+ * to the start of the mac header and where "direct packet access"
+ * is not available.
+ *
+ * Return
+ * 0 on success, or a negative error in case of failure.
+ *
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -1870,7 +1894,8 @@ union bpf_attr {
FN(bind), \
FN(xdp_adjust_tail), \
FN(skb_get_xfrm_state), \
- FN(get_stack),
+ FN(get_stack), \
+ FN(skb_load_bytes_relative),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -1931,6 +1956,12 @@ enum bpf_adj_room_mode {
BPF_ADJ_ROOM_NET,
};
+/* Mode for BPF_FUNC_skb_load_bytes_relative helper. */
+enum bpf_hdr_start_off {
+ BPF_HDR_START_MAC,
+ BPF_HDR_START_NET,
+};
+
/* user accessible mirror of in-kernel sk_buff.
* new fields can only be added to the end of this structure
*/
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 09/12] bpf, mips64: remove ld_abs/ld_ind
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
Since LD_ABS/LD_IND instructions are now removed from the core and
reimplemented through a combination of inlined BPF instructions and
a slow-path helper, we can get rid of the complexity from mips64 JIT.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
arch/mips/net/ebpf_jit.c | 104 -----------------------------------------------
1 file changed, 104 deletions(-)
diff --git a/arch/mips/net/ebpf_jit.c b/arch/mips/net/ebpf_jit.c
index 3e2798b..7ba7df9 100644
--- a/arch/mips/net/ebpf_jit.c
+++ b/arch/mips/net/ebpf_jit.c
@@ -1267,110 +1267,6 @@ static int build_one_insn(const struct bpf_insn *insn, struct jit_ctx *ctx,
return -EINVAL;
break;
- case BPF_LD | BPF_B | BPF_ABS:
- case BPF_LD | BPF_H | BPF_ABS:
- case BPF_LD | BPF_W | BPF_ABS:
- case BPF_LD | BPF_DW | BPF_ABS:
- ctx->flags |= EBPF_SAVE_RA;
-
- gen_imm_to_reg(insn, MIPS_R_A1, ctx);
- emit_instr(ctx, addiu, MIPS_R_A2, MIPS_R_ZERO, size_to_len(insn));
-
- if (insn->imm < 0) {
- emit_const_to_reg(ctx, MIPS_R_T9, (u64)bpf_internal_load_pointer_neg_helper);
- } else {
- emit_const_to_reg(ctx, MIPS_R_T9, (u64)ool_skb_header_pointer);
- emit_instr(ctx, daddiu, MIPS_R_A3, MIPS_R_SP, ctx->tmp_offset);
- }
- goto ld_skb_common;
-
- case BPF_LD | BPF_B | BPF_IND:
- case BPF_LD | BPF_H | BPF_IND:
- case BPF_LD | BPF_W | BPF_IND:
- case BPF_LD | BPF_DW | BPF_IND:
- ctx->flags |= EBPF_SAVE_RA;
- src = ebpf_to_mips_reg(ctx, insn, src_reg_no_fp);
- if (src < 0)
- return src;
- ts = get_reg_val_type(ctx, this_idx, insn->src_reg);
- if (ts == REG_32BIT_ZERO_EX) {
- /* sign extend */
- emit_instr(ctx, sll, MIPS_R_A1, src, 0);
- src = MIPS_R_A1;
- }
- if (insn->imm >= S16_MIN && insn->imm <= S16_MAX) {
- emit_instr(ctx, daddiu, MIPS_R_A1, src, insn->imm);
- } else {
- gen_imm_to_reg(insn, MIPS_R_AT, ctx);
- emit_instr(ctx, daddu, MIPS_R_A1, MIPS_R_AT, src);
- }
- /* truncate to 32-bit int */
- emit_instr(ctx, sll, MIPS_R_A1, MIPS_R_A1, 0);
- emit_instr(ctx, daddiu, MIPS_R_A3, MIPS_R_SP, ctx->tmp_offset);
- emit_instr(ctx, slt, MIPS_R_AT, MIPS_R_A1, MIPS_R_ZERO);
-
- emit_const_to_reg(ctx, MIPS_R_T8, (u64)bpf_internal_load_pointer_neg_helper);
- emit_const_to_reg(ctx, MIPS_R_T9, (u64)ool_skb_header_pointer);
- emit_instr(ctx, addiu, MIPS_R_A2, MIPS_R_ZERO, size_to_len(insn));
- emit_instr(ctx, movn, MIPS_R_T9, MIPS_R_T8, MIPS_R_AT);
-
-ld_skb_common:
- emit_instr(ctx, jalr, MIPS_R_RA, MIPS_R_T9);
- /* delay slot move */
- emit_instr(ctx, daddu, MIPS_R_A0, MIPS_R_S0, MIPS_R_ZERO);
-
- /* Check the error value */
- b_off = b_imm(exit_idx, ctx);
- if (is_bad_offset(b_off)) {
- target = j_target(ctx, exit_idx);
- if (target == (unsigned int)-1)
- return -E2BIG;
-
- if (!(ctx->offsets[this_idx] & OFFSETS_B_CONV)) {
- ctx->offsets[this_idx] |= OFFSETS_B_CONV;
- ctx->long_b_conversion = 1;
- }
- emit_instr(ctx, bne, MIPS_R_V0, MIPS_R_ZERO, 4 * 3);
- emit_instr(ctx, nop);
- emit_instr(ctx, j, target);
- emit_instr(ctx, nop);
- } else {
- emit_instr(ctx, beq, MIPS_R_V0, MIPS_R_ZERO, b_off);
- emit_instr(ctx, nop);
- }
-
-#ifdef __BIG_ENDIAN
- need_swap = false;
-#else
- need_swap = true;
-#endif
- dst = MIPS_R_V0;
- switch (BPF_SIZE(insn->code)) {
- case BPF_B:
- emit_instr(ctx, lbu, dst, 0, MIPS_R_V0);
- break;
- case BPF_H:
- emit_instr(ctx, lhu, dst, 0, MIPS_R_V0);
- if (need_swap)
- emit_instr(ctx, wsbh, dst, dst);
- break;
- case BPF_W:
- emit_instr(ctx, lw, dst, 0, MIPS_R_V0);
- if (need_swap) {
- emit_instr(ctx, wsbh, dst, dst);
- emit_instr(ctx, rotr, dst, dst, 16);
- }
- break;
- case BPF_DW:
- emit_instr(ctx, ld, dst, 0, MIPS_R_V0);
- if (need_swap) {
- emit_instr(ctx, dsbh, dst, dst);
- emit_instr(ctx, dshd, dst, dst);
- }
- break;
- }
-
- break;
case BPF_ALU | BPF_END | BPF_FROM_BE:
case BPF_ALU | BPF_END | BPF_FROM_LE:
dst = ebpf_to_mips_reg(ctx, insn, dst_reg);
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 10/12] bpf, ppc64: remove ld_abs/ld_ind
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
Since LD_ABS/LD_IND instructions are now removed from the core and
reimplemented through a combination of inlined BPF instructions and
a slow-path helper, we can get rid of the complexity from ppc64 JIT.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Tested-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
---
arch/powerpc/net/Makefile | 2 +-
arch/powerpc/net/bpf_jit64.h | 37 ++------
arch/powerpc/net/bpf_jit_asm64.S | 180 --------------------------------------
arch/powerpc/net/bpf_jit_comp64.c | 109 +----------------------
4 files changed, 11 insertions(+), 317 deletions(-)
delete mode 100644 arch/powerpc/net/bpf_jit_asm64.S
diff --git a/arch/powerpc/net/Makefile b/arch/powerpc/net/Makefile
index 02d369c..809f019 100644
--- a/arch/powerpc/net/Makefile
+++ b/arch/powerpc/net/Makefile
@@ -3,7 +3,7 @@
# Arch-specific network modules
#
ifeq ($(CONFIG_PPC64),y)
-obj-$(CONFIG_BPF_JIT) += bpf_jit_asm64.o bpf_jit_comp64.o
+obj-$(CONFIG_BPF_JIT) += bpf_jit_comp64.o
else
obj-$(CONFIG_BPF_JIT) += bpf_jit_asm.o bpf_jit_comp.o
endif
diff --git a/arch/powerpc/net/bpf_jit64.h b/arch/powerpc/net/bpf_jit64.h
index 8bdef7e..3609be4 100644
--- a/arch/powerpc/net/bpf_jit64.h
+++ b/arch/powerpc/net/bpf_jit64.h
@@ -20,7 +20,7 @@
* with our redzone usage.
*
* [ prev sp ] <-------------
- * [ nv gpr save area ] 8*8 |
+ * [ nv gpr save area ] 6*8 |
* [ tail_call_cnt ] 8 |
* [ local_tmp_var ] 8 |
* fp (r31) --> [ ebpf stack space ] upto 512 |
@@ -28,8 +28,8 @@
* sp (r1) ---> [ stack pointer ] --------------
*/
-/* for gpr non volatile registers BPG_REG_6 to 10, plus skb cache registers */
-#define BPF_PPC_STACK_SAVE (8*8)
+/* for gpr non volatile registers BPG_REG_6 to 10 */
+#define BPF_PPC_STACK_SAVE (6*8)
/* for bpf JIT code internal usage */
#define BPF_PPC_STACK_LOCALS 16
/* stack frame excluding BPF stack, ensure this is quadword aligned */
@@ -39,10 +39,8 @@
#ifndef __ASSEMBLY__
/* BPF register usage */
-#define SKB_HLEN_REG (MAX_BPF_JIT_REG + 0)
-#define SKB_DATA_REG (MAX_BPF_JIT_REG + 1)
-#define TMP_REG_1 (MAX_BPF_JIT_REG + 2)
-#define TMP_REG_2 (MAX_BPF_JIT_REG + 3)
+#define TMP_REG_1 (MAX_BPF_JIT_REG + 0)
+#define TMP_REG_2 (MAX_BPF_JIT_REG + 1)
/* BPF to ppc register mappings */
static const int b2p[] = {
@@ -63,40 +61,23 @@ static const int b2p[] = {
[BPF_REG_FP] = 31,
/* eBPF jit internal registers */
[BPF_REG_AX] = 2,
- [SKB_HLEN_REG] = 25,
- [SKB_DATA_REG] = 26,
[TMP_REG_1] = 9,
[TMP_REG_2] = 10
};
-/* PPC NVR range -- update this if we ever use NVRs below r24 */
-#define BPF_PPC_NVR_MIN 24
-
-/* Assembly helpers */
-#define DECLARE_LOAD_FUNC(func) u64 func(u64 r3, u64 r4); \
- u64 func##_negative_offset(u64 r3, u64 r4); \
- u64 func##_positive_offset(u64 r3, u64 r4);
-
-DECLARE_LOAD_FUNC(sk_load_word);
-DECLARE_LOAD_FUNC(sk_load_half);
-DECLARE_LOAD_FUNC(sk_load_byte);
-
-#define CHOOSE_LOAD_FUNC(imm, func) \
- (imm < 0 ? \
- (imm >= SKF_LL_OFF ? func##_negative_offset : func) : \
- func##_positive_offset)
+/* PPC NVR range -- update this if we ever use NVRs below r27 */
+#define BPF_PPC_NVR_MIN 27
#define SEEN_FUNC 0x1000 /* might call external helpers */
#define SEEN_STACK 0x2000 /* uses BPF stack */
-#define SEEN_SKB 0x4000 /* uses sk_buff */
-#define SEEN_TAILCALL 0x8000 /* uses tail calls */
+#define SEEN_TAILCALL 0x4000 /* uses tail calls */
struct codegen_context {
/*
* This is used to track register usage as well
* as calls to external helpers.
* - register usage is tracked with corresponding
- * bits (r3-r10 and r25-r31)
+ * bits (r3-r10 and r27-r31)
* - rest of the bits can be used to track other
* things -- for now, we use bits 16 to 23
* encoded in SEEN_* macros above
diff --git a/arch/powerpc/net/bpf_jit_asm64.S b/arch/powerpc/net/bpf_jit_asm64.S
deleted file mode 100644
index 7e4c514..0000000
--- a/arch/powerpc/net/bpf_jit_asm64.S
+++ /dev/null
@@ -1,180 +0,0 @@
-/*
- * bpf_jit_asm64.S: Packet/header access helper functions
- * for PPC64 BPF compiler.
- *
- * Copyright 2016, Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
- * IBM Corporation
- *
- * Based on bpf_jit_asm.S by Matt Evans
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; version 2
- * of the License.
- */
-
-#include <asm/ppc_asm.h>
-#include <asm/ptrace.h>
-#include "bpf_jit64.h"
-
-/*
- * All of these routines are called directly from generated code,
- * with the below register usage:
- * r27 skb pointer (ctx)
- * r25 skb header length
- * r26 skb->data pointer
- * r4 offset
- *
- * Result is passed back in:
- * r8 data read in host endian format (accumulator)
- *
- * r9 is used as a temporary register
- */
-
-#define r_skb r27
-#define r_hlen r25
-#define r_data r26
-#define r_off r4
-#define r_val r8
-#define r_tmp r9
-
-_GLOBAL_TOC(sk_load_word)
- cmpdi r_off, 0
- blt bpf_slow_path_word_neg
- b sk_load_word_positive_offset
-
-_GLOBAL_TOC(sk_load_word_positive_offset)
- /* Are we accessing past headlen? */
- subi r_tmp, r_hlen, 4
- cmpd r_tmp, r_off
- blt bpf_slow_path_word
- /* Nope, just hitting the header. cr0 here is eq or gt! */
- LWZX_BE r_val, r_data, r_off
- blr /* Return success, cr0 != LT */
-
-_GLOBAL_TOC(sk_load_half)
- cmpdi r_off, 0
- blt bpf_slow_path_half_neg
- b sk_load_half_positive_offset
-
-_GLOBAL_TOC(sk_load_half_positive_offset)
- subi r_tmp, r_hlen, 2
- cmpd r_tmp, r_off
- blt bpf_slow_path_half
- LHZX_BE r_val, r_data, r_off
- blr
-
-_GLOBAL_TOC(sk_load_byte)
- cmpdi r_off, 0
- blt bpf_slow_path_byte_neg
- b sk_load_byte_positive_offset
-
-_GLOBAL_TOC(sk_load_byte_positive_offset)
- cmpd r_hlen, r_off
- ble bpf_slow_path_byte
- lbzx r_val, r_data, r_off
- blr
-
-/*
- * Call out to skb_copy_bits:
- * Allocate a new stack frame here to remain ABI-compliant in
- * stashing LR.
- */
-#define bpf_slow_path_common(SIZE) \
- mflr r0; \
- std r0, PPC_LR_STKOFF(r1); \
- stdu r1, -(STACK_FRAME_MIN_SIZE + BPF_PPC_STACK_LOCALS)(r1); \
- mr r3, r_skb; \
- /* r4 = r_off as passed */ \
- addi r5, r1, STACK_FRAME_MIN_SIZE; \
- li r6, SIZE; \
- bl skb_copy_bits; \
- nop; \
- /* save r5 */ \
- addi r5, r1, STACK_FRAME_MIN_SIZE; \
- /* r3 = 0 on success */ \
- addi r1, r1, STACK_FRAME_MIN_SIZE + BPF_PPC_STACK_LOCALS; \
- ld r0, PPC_LR_STKOFF(r1); \
- mtlr r0; \
- cmpdi r3, 0; \
- blt bpf_error; /* cr0 = LT */
-
-bpf_slow_path_word:
- bpf_slow_path_common(4)
- /* Data value is on stack, and cr0 != LT */
- LWZX_BE r_val, 0, r5
- blr
-
-bpf_slow_path_half:
- bpf_slow_path_common(2)
- LHZX_BE r_val, 0, r5
- blr
-
-bpf_slow_path_byte:
- bpf_slow_path_common(1)
- lbzx r_val, 0, r5
- blr
-
-/*
- * Call out to bpf_internal_load_pointer_neg_helper
- */
-#define sk_negative_common(SIZE) \
- mflr r0; \
- std r0, PPC_LR_STKOFF(r1); \
- stdu r1, -STACK_FRAME_MIN_SIZE(r1); \
- mr r3, r_skb; \
- /* r4 = r_off, as passed */ \
- li r5, SIZE; \
- bl bpf_internal_load_pointer_neg_helper; \
- nop; \
- addi r1, r1, STACK_FRAME_MIN_SIZE; \
- ld r0, PPC_LR_STKOFF(r1); \
- mtlr r0; \
- /* R3 != 0 on success */ \
- cmpldi r3, 0; \
- beq bpf_error_slow; /* cr0 = EQ */
-
-bpf_slow_path_word_neg:
- lis r_tmp, -32 /* SKF_LL_OFF */
- cmpd r_off, r_tmp /* addr < SKF_* */
- blt bpf_error /* cr0 = LT */
- b sk_load_word_negative_offset
-
-_GLOBAL_TOC(sk_load_word_negative_offset)
- sk_negative_common(4)
- LWZX_BE r_val, 0, r3
- blr
-
-bpf_slow_path_half_neg:
- lis r_tmp, -32 /* SKF_LL_OFF */
- cmpd r_off, r_tmp /* addr < SKF_* */
- blt bpf_error /* cr0 = LT */
- b sk_load_half_negative_offset
-
-_GLOBAL_TOC(sk_load_half_negative_offset)
- sk_negative_common(2)
- LHZX_BE r_val, 0, r3
- blr
-
-bpf_slow_path_byte_neg:
- lis r_tmp, -32 /* SKF_LL_OFF */
- cmpd r_off, r_tmp /* addr < SKF_* */
- blt bpf_error /* cr0 = LT */
- b sk_load_byte_negative_offset
-
-_GLOBAL_TOC(sk_load_byte_negative_offset)
- sk_negative_common(1)
- lbzx r_val, 0, r3
- blr
-
-bpf_error_slow:
- /* fabricate a cr0 = lt */
- li r_tmp, -1
- cmpdi r_tmp, 0
-bpf_error:
- /*
- * Entered with cr0 = lt
- * Generated code will 'blt epilogue', returning 0.
- */
- li r_val, 0
- blr
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 0ef3d95..1bdb1af 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -59,7 +59,7 @@ static inline bool bpf_has_stack_frame(struct codegen_context *ctx)
* [ prev sp ] <-------------
* [ ... ] |
* sp (r1) ---> [ stack pointer ] --------------
- * [ nv gpr save area ] 8*8
+ * [ nv gpr save area ] 6*8
* [ tail_call_cnt ] 8
* [ local_tmp_var ] 8
* [ unused red zone ] 208 bytes protected
@@ -88,21 +88,6 @@ static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg)
BUG();
}
-static void bpf_jit_emit_skb_loads(u32 *image, struct codegen_context *ctx)
-{
- /*
- * Load skb->len and skb->data_len
- * r3 points to skb
- */
- PPC_LWZ(b2p[SKB_HLEN_REG], 3, offsetof(struct sk_buff, len));
- PPC_LWZ(b2p[TMP_REG_1], 3, offsetof(struct sk_buff, data_len));
- /* header_len = len - data_len */
- PPC_SUB(b2p[SKB_HLEN_REG], b2p[SKB_HLEN_REG], b2p[TMP_REG_1]);
-
- /* skb->data pointer */
- PPC_BPF_LL(b2p[SKB_DATA_REG], 3, offsetof(struct sk_buff, data));
-}
-
static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
{
int i;
@@ -145,18 +130,6 @@ static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
if (bpf_is_seen_register(ctx, i))
PPC_BPF_STL(b2p[i], 1, bpf_jit_stack_offsetof(ctx, b2p[i]));
- /*
- * Save additional non-volatile regs if we cache skb
- * Also, setup skb data
- */
- if (ctx->seen & SEEN_SKB) {
- PPC_BPF_STL(b2p[SKB_HLEN_REG], 1,
- bpf_jit_stack_offsetof(ctx, b2p[SKB_HLEN_REG]));
- PPC_BPF_STL(b2p[SKB_DATA_REG], 1,
- bpf_jit_stack_offsetof(ctx, b2p[SKB_DATA_REG]));
- bpf_jit_emit_skb_loads(image, ctx);
- }
-
/* Setup frame pointer to point to the bpf stack area */
if (bpf_is_seen_register(ctx, BPF_REG_FP))
PPC_ADDI(b2p[BPF_REG_FP], 1,
@@ -172,14 +145,6 @@ static void bpf_jit_emit_common_epilogue(u32 *image, struct codegen_context *ctx
if (bpf_is_seen_register(ctx, i))
PPC_BPF_LL(b2p[i], 1, bpf_jit_stack_offsetof(ctx, b2p[i]));
- /* Restore non-volatile registers used for skb cache */
- if (ctx->seen & SEEN_SKB) {
- PPC_BPF_LL(b2p[SKB_HLEN_REG], 1,
- bpf_jit_stack_offsetof(ctx, b2p[SKB_HLEN_REG]));
- PPC_BPF_LL(b2p[SKB_DATA_REG], 1,
- bpf_jit_stack_offsetof(ctx, b2p[SKB_DATA_REG]));
- }
-
/* Tear down our stack frame */
if (bpf_has_stack_frame(ctx)) {
PPC_ADDI(1, 1, BPF_PPC_STACKFRAME + ctx->stack_size);
@@ -753,23 +718,10 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
ctx->seen |= SEEN_FUNC;
func = (u8 *) __bpf_call_base + imm;
- /* Save skb pointer if we need to re-cache skb data */
- if ((ctx->seen & SEEN_SKB) &&
- bpf_helper_changes_pkt_data(func))
- PPC_BPF_STL(3, 1, bpf_jit_stack_local(ctx));
-
bpf_jit_emit_func_call(image, ctx, (u64)func);
/* move return value from r3 to BPF_REG_0 */
PPC_MR(b2p[BPF_REG_0], 3);
-
- /* refresh skb cache */
- if ((ctx->seen & SEEN_SKB) &&
- bpf_helper_changes_pkt_data(func)) {
- /* reload skb pointer to r3 */
- PPC_BPF_LL(3, 1, bpf_jit_stack_local(ctx));
- bpf_jit_emit_skb_loads(image, ctx);
- }
break;
/*
@@ -887,65 +839,6 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
break;
/*
- * Loads from packet header/data
- * Assume 32-bit input value in imm and X (src_reg)
- */
-
- /* Absolute loads */
- case BPF_LD | BPF_W | BPF_ABS:
- func = (u8 *)CHOOSE_LOAD_FUNC(imm, sk_load_word);
- goto common_load_abs;
- case BPF_LD | BPF_H | BPF_ABS:
- func = (u8 *)CHOOSE_LOAD_FUNC(imm, sk_load_half);
- goto common_load_abs;
- case BPF_LD | BPF_B | BPF_ABS:
- func = (u8 *)CHOOSE_LOAD_FUNC(imm, sk_load_byte);
-common_load_abs:
- /*
- * Load from [imm]
- * Load into r4, which can just be passed onto
- * skb load helpers as the second parameter
- */
- PPC_LI32(4, imm);
- goto common_load;
-
- /* Indirect loads */
- case BPF_LD | BPF_W | BPF_IND:
- func = (u8 *)sk_load_word;
- goto common_load_ind;
- case BPF_LD | BPF_H | BPF_IND:
- func = (u8 *)sk_load_half;
- goto common_load_ind;
- case BPF_LD | BPF_B | BPF_IND:
- func = (u8 *)sk_load_byte;
-common_load_ind:
- /*
- * Load from [src_reg + imm]
- * Treat src_reg as a 32-bit value
- */
- PPC_EXTSW(4, src_reg);
- if (imm) {
- if (imm >= -32768 && imm < 32768)
- PPC_ADDI(4, 4, IMM_L(imm));
- else {
- PPC_LI32(b2p[TMP_REG_1], imm);
- PPC_ADD(4, 4, b2p[TMP_REG_1]);
- }
- }
-
-common_load:
- ctx->seen |= SEEN_SKB;
- ctx->seen |= SEEN_FUNC;
- bpf_jit_emit_func_call(image, ctx, (u64)func);
-
- /*
- * Helper returns 'lt' condition on error, and an
- * appropriate return value in BPF_REG_0
- */
- PPC_BCC(COND_LT, exit_addr);
- break;
-
- /*
* Tail call
*/
case BPF_JMP | BPF_TAIL_CALL:
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 08/12] bpf, arm32: remove ld_abs/ld_ind
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
Since LD_ABS/LD_IND instructions are now removed from the core and
reimplemented through a combination of inlined BPF instructions and
a slow-path helper, we can get rid of the complexity from arm32 JIT.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
arch/arm/net/bpf_jit_32.c | 77 -----------------------------------------------
1 file changed, 77 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index b5030e1..82689b9 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1452,83 +1452,6 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
emit(ARM_LDR_I(rn, ARM_SP, STACK_VAR(src_lo)), ctx);
emit_ldx_r(dst, rn, dstk, off, ctx, BPF_SIZE(code));
break;
- /* R0 = ntohx(*(size *)(((struct sk_buff *)R6)->data + imm)) */
- case BPF_LD | BPF_ABS | BPF_W:
- case BPF_LD | BPF_ABS | BPF_H:
- case BPF_LD | BPF_ABS | BPF_B:
- /* R0 = ntohx(*(size *)(((struct sk_buff *)R6)->data + src + imm)) */
- case BPF_LD | BPF_IND | BPF_W:
- case BPF_LD | BPF_IND | BPF_H:
- case BPF_LD | BPF_IND | BPF_B:
- {
- const u8 r4 = bpf2a32[BPF_REG_6][1]; /* r4 = ptr to sk_buff */
- const u8 r0 = bpf2a32[BPF_REG_0][1]; /*r0: struct sk_buff *skb*/
- /* rtn value */
- const u8 r1 = bpf2a32[BPF_REG_0][0]; /* r1: int k */
- const u8 r2 = bpf2a32[BPF_REG_1][1]; /* r2: unsigned int size */
- const u8 r3 = bpf2a32[BPF_REG_1][0]; /* r3: void *buffer */
- const u8 r6 = bpf2a32[TMP_REG_1][1]; /* r6: void *(*func)(..) */
- int size;
-
- /* Setting up first argument */
- emit(ARM_MOV_R(r0, r4), ctx);
-
- /* Setting up second argument */
- emit_a32_mov_i(r1, imm, false, ctx);
- if (BPF_MODE(code) == BPF_IND)
- emit_a32_alu_r(r1, src_lo, false, sstk, ctx,
- false, false, BPF_ADD);
-
- /* Setting up third argument */
- switch (BPF_SIZE(code)) {
- case BPF_W:
- size = 4;
- break;
- case BPF_H:
- size = 2;
- break;
- case BPF_B:
- size = 1;
- break;
- default:
- return -EINVAL;
- }
- emit_a32_mov_i(r2, size, false, ctx);
-
- /* Setting up fourth argument */
- emit(ARM_ADD_I(r3, ARM_SP, imm8m(SKB_BUFFER)), ctx);
-
- /* Setting up function pointer to call */
- emit_a32_mov_i(r6, (unsigned int)bpf_load_pointer, false, ctx);
- emit_blx_r(r6, ctx);
-
- emit(ARM_EOR_R(r1, r1, r1), ctx);
- /* Check if return address is NULL or not.
- * if NULL then jump to epilogue
- * else continue to load the value from retn address
- */
- emit(ARM_CMP_I(r0, 0), ctx);
- jmp_offset = epilogue_offset(ctx);
- check_imm24(jmp_offset);
- _emit(ARM_COND_EQ, ARM_B(jmp_offset), ctx);
-
- /* Load value from the address */
- switch (BPF_SIZE(code)) {
- case BPF_W:
- emit(ARM_LDR_I(r0, r0, 0), ctx);
- emit_rev32(r0, r0, ctx);
- break;
- case BPF_H:
- emit(ARM_LDRH_I(r0, r0, 0), ctx);
- emit_rev16(r0, r0, ctx);
- break;
- case BPF_B:
- emit(ARM_LDRB_I(r0, r0, 0), ctx);
- /* No need to reverse */
- break;
- }
- break;
- }
/* ST: *(size *)(dst + off) = imm */
case BPF_ST | BPF_MEM | BPF_W:
case BPF_ST | BPF_MEM | BPF_H:
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 06/12] bpf, arm64: remove ld_abs/ld_ind
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
Since LD_ABS/LD_IND instructions are now removed from the core and
reimplemented through a combination of inlined BPF instructions and
a slow-path helper, we can get rid of the complexity from arm64 JIT.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
arch/arm64/net/bpf_jit_comp.c | 65 -------------------------------------------
1 file changed, 65 deletions(-)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index a933504..0b40c8f 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -723,71 +723,6 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
emit(A64_CBNZ(0, tmp3, jmp_offset), ctx);
break;
- /* R0 = ntohx(*(size *)(((struct sk_buff *)R6)->data + imm)) */
- case BPF_LD | BPF_ABS | BPF_W:
- case BPF_LD | BPF_ABS | BPF_H:
- case BPF_LD | BPF_ABS | BPF_B:
- /* R0 = ntohx(*(size *)(((struct sk_buff *)R6)->data + src + imm)) */
- case BPF_LD | BPF_IND | BPF_W:
- case BPF_LD | BPF_IND | BPF_H:
- case BPF_LD | BPF_IND | BPF_B:
- {
- const u8 r0 = bpf2a64[BPF_REG_0]; /* r0 = return value */
- const u8 r6 = bpf2a64[BPF_REG_6]; /* r6 = pointer to sk_buff */
- const u8 fp = bpf2a64[BPF_REG_FP];
- const u8 r1 = bpf2a64[BPF_REG_1]; /* r1: struct sk_buff *skb */
- const u8 r2 = bpf2a64[BPF_REG_2]; /* r2: int k */
- const u8 r3 = bpf2a64[BPF_REG_3]; /* r3: unsigned int size */
- const u8 r4 = bpf2a64[BPF_REG_4]; /* r4: void *buffer */
- const u8 r5 = bpf2a64[BPF_REG_5]; /* r5: void *(*func)(...) */
- int size;
-
- emit(A64_MOV(1, r1, r6), ctx);
- emit_a64_mov_i(0, r2, imm, ctx);
- if (BPF_MODE(code) == BPF_IND)
- emit(A64_ADD(0, r2, r2, src), ctx);
- switch (BPF_SIZE(code)) {
- case BPF_W:
- size = 4;
- break;
- case BPF_H:
- size = 2;
- break;
- case BPF_B:
- size = 1;
- break;
- default:
- return -EINVAL;
- }
- emit_a64_mov_i64(r3, size, ctx);
- emit(A64_SUB_I(1, r4, fp, ctx->stack_size), ctx);
- emit_a64_mov_i64(r5, (unsigned long)bpf_load_pointer, ctx);
- emit(A64_BLR(r5), ctx);
- emit(A64_MOV(1, r0, A64_R(0)), ctx);
-
- jmp_offset = epilogue_offset(ctx);
- check_imm19(jmp_offset);
- emit(A64_CBZ(1, r0, jmp_offset), ctx);
- emit(A64_MOV(1, r5, r0), ctx);
- switch (BPF_SIZE(code)) {
- case BPF_W:
- emit(A64_LDR32(r0, r5, A64_ZR), ctx);
-#ifndef CONFIG_CPU_BIG_ENDIAN
- emit(A64_REV32(0, r0, r0), ctx);
-#endif
- break;
- case BPF_H:
- emit(A64_LDRH(r0, r5, A64_ZR), ctx);
-#ifndef CONFIG_CPU_BIG_ENDIAN
- emit(A64_REV16(0, r0, r0), ctx);
-#endif
- break;
- case BPF_B:
- emit(A64_LDRB(r0, r5, A64_ZR), ctx);
- break;
- }
- break;
- }
default:
pr_err_once("unknown opcode %02x\n", code);
return -EINVAL;
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 05/12] bpf, x64: remove ld_abs/ld_ind
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
Since LD_ABS/LD_IND instructions are now removed from the core and
reimplemented through a combination of inlined BPF instructions and
a slow-path helper, we can get rid of the complexity from x64 JIT.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
arch/x86/net/Makefile | 4 +-
arch/x86/net/bpf_jit.S | 154 --------------------------------------------
arch/x86/net/bpf_jit_comp.c | 144 ++---------------------------------------
3 files changed, 5 insertions(+), 297 deletions(-)
delete mode 100644 arch/x86/net/bpf_jit.S
diff --git a/arch/x86/net/Makefile b/arch/x86/net/Makefile
index fefb4b6..20277db 100644
--- a/arch/x86/net/Makefile
+++ b/arch/x86/net/Makefile
@@ -1,6 +1,4 @@
#
# Arch-specific network modules
#
-OBJECT_FILES_NON_STANDARD_bpf_jit.o += y
-
-obj-$(CONFIG_BPF_JIT) += bpf_jit.o bpf_jit_comp.o
+obj-$(CONFIG_BPF_JIT) += bpf_jit_comp.o
diff --git a/arch/x86/net/bpf_jit.S b/arch/x86/net/bpf_jit.S
deleted file mode 100644
index b33093f..0000000
--- a/arch/x86/net/bpf_jit.S
+++ /dev/null
@@ -1,154 +0,0 @@
-/* bpf_jit.S : BPF JIT helper functions
- *
- * Copyright (C) 2011 Eric Dumazet (eric.dumazet@gmail.com)
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; version 2
- * of the License.
- */
-#include <linux/linkage.h>
-#include <asm/frame.h>
-
-/*
- * Calling convention :
- * rbx : skb pointer (callee saved)
- * esi : offset of byte(s) to fetch in skb (can be scratched)
- * r10 : copy of skb->data
- * r9d : hlen = skb->len - skb->data_len
- */
-#define SKBDATA %r10
-#define SKF_MAX_NEG_OFF $(-0x200000) /* SKF_LL_OFF from filter.h */
-
-#define FUNC(name) \
- .globl name; \
- .type name, @function; \
- name:
-
-FUNC(sk_load_word)
- test %esi,%esi
- js bpf_slow_path_word_neg
-
-FUNC(sk_load_word_positive_offset)
- mov %r9d,%eax # hlen
- sub %esi,%eax # hlen - offset
- cmp $3,%eax
- jle bpf_slow_path_word
- mov (SKBDATA,%rsi),%eax
- bswap %eax /* ntohl() */
- ret
-
-FUNC(sk_load_half)
- test %esi,%esi
- js bpf_slow_path_half_neg
-
-FUNC(sk_load_half_positive_offset)
- mov %r9d,%eax
- sub %esi,%eax # hlen - offset
- cmp $1,%eax
- jle bpf_slow_path_half
- movzwl (SKBDATA,%rsi),%eax
- rol $8,%ax # ntohs()
- ret
-
-FUNC(sk_load_byte)
- test %esi,%esi
- js bpf_slow_path_byte_neg
-
-FUNC(sk_load_byte_positive_offset)
- cmp %esi,%r9d /* if (offset >= hlen) goto bpf_slow_path_byte */
- jle bpf_slow_path_byte
- movzbl (SKBDATA,%rsi),%eax
- ret
-
-/* rsi contains offset and can be scratched */
-#define bpf_slow_path_common(LEN) \
- lea 32(%rbp), %rdx;\
- FRAME_BEGIN; \
- mov %rbx, %rdi; /* arg1 == skb */ \
- push %r9; \
- push SKBDATA; \
-/* rsi already has offset */ \
- mov $LEN,%ecx; /* len */ \
- call skb_copy_bits; \
- test %eax,%eax; \
- pop SKBDATA; \
- pop %r9; \
- FRAME_END
-
-
-bpf_slow_path_word:
- bpf_slow_path_common(4)
- js bpf_error
- mov 32(%rbp),%eax
- bswap %eax
- ret
-
-bpf_slow_path_half:
- bpf_slow_path_common(2)
- js bpf_error
- mov 32(%rbp),%ax
- rol $8,%ax
- movzwl %ax,%eax
- ret
-
-bpf_slow_path_byte:
- bpf_slow_path_common(1)
- js bpf_error
- movzbl 32(%rbp),%eax
- ret
-
-#define sk_negative_common(SIZE) \
- FRAME_BEGIN; \
- mov %rbx, %rdi; /* arg1 == skb */ \
- push %r9; \
- push SKBDATA; \
-/* rsi already has offset */ \
- mov $SIZE,%edx; /* size */ \
- call bpf_internal_load_pointer_neg_helper; \
- test %rax,%rax; \
- pop SKBDATA; \
- pop %r9; \
- FRAME_END; \
- jz bpf_error
-
-bpf_slow_path_word_neg:
- cmp SKF_MAX_NEG_OFF, %esi /* test range */
- jl bpf_error /* offset lower -> error */
-
-FUNC(sk_load_word_negative_offset)
- sk_negative_common(4)
- mov (%rax), %eax
- bswap %eax
- ret
-
-bpf_slow_path_half_neg:
- cmp SKF_MAX_NEG_OFF, %esi
- jl bpf_error
-
-FUNC(sk_load_half_negative_offset)
- sk_negative_common(2)
- mov (%rax),%ax
- rol $8,%ax
- movzwl %ax,%eax
- ret
-
-bpf_slow_path_byte_neg:
- cmp SKF_MAX_NEG_OFF, %esi
- jl bpf_error
-
-FUNC(sk_load_byte_negative_offset)
- sk_negative_common(1)
- movzbl (%rax), %eax
- ret
-
-bpf_error:
-# force a return 0 from jit handler
- xor %eax,%eax
- mov (%rbp),%rbx
- mov 8(%rbp),%r13
- mov 16(%rbp),%r14
- mov 24(%rbp),%r15
- add $40, %rbp
- leaveq
- ret
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 1c3c81d..ce08b7b 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -17,15 +17,6 @@
#include <asm/set_memory.h>
#include <asm/nospec-branch.h>
-/*
- * Assembly code in arch/x86/net/bpf_jit.S
- */
-extern u8 sk_load_word[], sk_load_half[], sk_load_byte[];
-extern u8 sk_load_word_positive_offset[], sk_load_half_positive_offset[];
-extern u8 sk_load_byte_positive_offset[];
-extern u8 sk_load_word_negative_offset[], sk_load_half_negative_offset[];
-extern u8 sk_load_byte_negative_offset[];
-
static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
{
if (len == 1)
@@ -107,9 +98,6 @@ static int bpf_size_to_x86_bytes(int bpf_size)
#define X86_JLE 0x7E
#define X86_JG 0x7F
-#define CHOOSE_LOAD_FUNC(K, func) \
- ((int)K < 0 ? ((int)K >= SKF_LL_OFF ? func##_negative_offset : func) : func##_positive_offset)
-
/* Pick a register outside of BPF range for JIT internal work */
#define AUX_REG (MAX_BPF_JIT_REG + 1)
@@ -120,8 +108,8 @@ static int bpf_size_to_x86_bytes(int bpf_size)
* register in load/store instructions, it always needs an
* extra byte of encoding and is callee saved.
*
- * R9 caches skb->len - skb->data_len
- * R10 caches skb->data, and used for blinding (if enabled)
+ * Also x86-64 register R9 is unused. x86-64 register R10 is
+ * used for blinding (if enabled).
*/
static const int reg2hex[] = {
[BPF_REG_0] = 0, /* RAX */
@@ -196,19 +184,15 @@ static void jit_fill_hole(void *area, unsigned int size)
struct jit_context {
int cleanup_addr; /* Epilogue code offset */
- bool seen_ld_abs;
- bool seen_ax_reg;
};
/* Maximum number of bytes emitted while JITing one eBPF insn */
#define BPF_MAX_INSN_SIZE 128
#define BPF_INSN_SAFETY 64
-#define AUX_STACK_SPACE \
- (32 /* Space for RBX, R13, R14, R15 */ + \
- 8 /* Space for skb_copy_bits() buffer */)
+#define AUX_STACK_SPACE 40 /* Space for RBX, R13, R14, R15, tailcnt */
-#define PROLOGUE_SIZE 37
+#define PROLOGUE_SIZE 37
/*
* Emit x86-64 prologue code for BPF program and check its size.
@@ -232,20 +216,8 @@ static void emit_prologue(u8 **pprog, u32 stack_depth, bool ebpf_from_cbpf)
/* sub rbp, AUX_STACK_SPACE */
EMIT4(0x48, 0x83, 0xED, AUX_STACK_SPACE);
- /* All classic BPF filters use R6(rbx) save it */
-
/* mov qword ptr [rbp+0],rbx */
EMIT4(0x48, 0x89, 0x5D, 0);
-
- /*
- * bpf_convert_filter() maps classic BPF register X to R7 and uses R8
- * as temporary, so all tcpdump filters need to spill/fill R7(R13) and
- * R8(R14). R9(R15) spill could be made conditional, but there is only
- * one 'bpf_error' return path out of helper functions inside bpf_jit.S
- * The overhead of extra spill is negligible for any filter other
- * than synthetic ones. Therefore not worth adding complexity.
- */
-
/* mov qword ptr [rbp+8],r13 */
EMIT4(0x4C, 0x89, 0x6D, 8);
/* mov qword ptr [rbp+16],r14 */
@@ -353,27 +325,6 @@ static void emit_bpf_tail_call(u8 **pprog)
*pprog = prog;
}
-
-static void emit_load_skb_data_hlen(u8 **pprog)
-{
- u8 *prog = *pprog;
- int cnt = 0;
-
- /*
- * r9d = skb->len - skb->data_len (headlen)
- * r10 = skb->data
- */
- /* mov %r9d, off32(%rdi) */
- EMIT3_off32(0x44, 0x8b, 0x8f, offsetof(struct sk_buff, len));
-
- /* sub %r9d, off32(%rdi) */
- EMIT3_off32(0x44, 0x2b, 0x8f, offsetof(struct sk_buff, data_len));
-
- /* mov %r10, off32(%rdi) */
- EMIT3_off32(0x4c, 0x8b, 0x97, offsetof(struct sk_buff, data));
- *pprog = prog;
-}
-
static void emit_mov_imm32(u8 **pprog, bool sign_propagate,
u32 dst_reg, const u32 imm32)
{
@@ -462,8 +413,6 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
{
struct bpf_insn *insn = bpf_prog->insnsi;
int insn_cnt = bpf_prog->len;
- bool seen_ld_abs = ctx->seen_ld_abs | (oldproglen == 0);
- bool seen_ax_reg = ctx->seen_ax_reg | (oldproglen == 0);
bool seen_exit = false;
u8 temp[BPF_MAX_INSN_SIZE + BPF_INSN_SAFETY];
int i, cnt = 0;
@@ -473,9 +422,6 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
emit_prologue(&prog, bpf_prog->aux->stack_depth,
bpf_prog_was_classic(bpf_prog));
- if (seen_ld_abs)
- emit_load_skb_data_hlen(&prog);
-
for (i = 0; i < insn_cnt; i++, insn++) {
const s32 imm32 = insn->imm;
u32 dst_reg = insn->dst_reg;
@@ -483,13 +429,9 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
u8 b2 = 0, b3 = 0;
s64 jmp_offset;
u8 jmp_cond;
- bool reload_skb_data;
int ilen;
u8 *func;
- if (dst_reg == BPF_REG_AX || src_reg == BPF_REG_AX)
- ctx->seen_ax_reg = seen_ax_reg = true;
-
switch (insn->code) {
/* ALU */
case BPF_ALU | BPF_ADD | BPF_X:
@@ -916,36 +858,12 @@ xadd: if (is_imm8(insn->off))
case BPF_JMP | BPF_CALL:
func = (u8 *) __bpf_call_base + imm32;
jmp_offset = func - (image + addrs[i]);
- if (seen_ld_abs) {
- reload_skb_data = bpf_helper_changes_pkt_data(func);
- if (reload_skb_data) {
- EMIT1(0x57); /* push %rdi */
- jmp_offset += 22; /* pop, mov, sub, mov */
- } else {
- EMIT2(0x41, 0x52); /* push %r10 */
- EMIT2(0x41, 0x51); /* push %r9 */
- /*
- * We need to adjust jmp offset, since
- * pop %r9, pop %r10 take 4 bytes after call insn
- */
- jmp_offset += 4;
- }
- }
if (!imm32 || !is_simm32(jmp_offset)) {
pr_err("unsupported BPF func %d addr %p image %p\n",
imm32, func, image);
return -EINVAL;
}
EMIT1_off32(0xE8, jmp_offset);
- if (seen_ld_abs) {
- if (reload_skb_data) {
- EMIT1(0x5F); /* pop %rdi */
- emit_load_skb_data_hlen(&prog);
- } else {
- EMIT2(0x41, 0x59); /* pop %r9 */
- EMIT2(0x41, 0x5A); /* pop %r10 */
- }
- }
break;
case BPF_JMP | BPF_TAIL_CALL:
@@ -1080,60 +998,6 @@ xadd: if (is_imm8(insn->off))
}
break;
- case BPF_LD | BPF_IND | BPF_W:
- func = sk_load_word;
- goto common_load;
- case BPF_LD | BPF_ABS | BPF_W:
- func = CHOOSE_LOAD_FUNC(imm32, sk_load_word);
-common_load:
- ctx->seen_ld_abs = seen_ld_abs = true;
- jmp_offset = func - (image + addrs[i]);
- if (!func || !is_simm32(jmp_offset)) {
- pr_err("unsupported BPF func %d addr %p image %p\n",
- imm32, func, image);
- return -EINVAL;
- }
- if (BPF_MODE(insn->code) == BPF_ABS) {
- /* mov %esi, imm32 */
- EMIT1_off32(0xBE, imm32);
- } else {
- /* mov %rsi, src_reg */
- EMIT_mov(BPF_REG_2, src_reg);
- if (imm32) {
- if (is_imm8(imm32))
- /* add %esi, imm8 */
- EMIT3(0x83, 0xC6, imm32);
- else
- /* add %esi, imm32 */
- EMIT2_off32(0x81, 0xC6, imm32);
- }
- }
- /*
- * skb pointer is in R6 (%rbx), it will be copied into
- * %rdi if skb_copy_bits() call is necessary.
- * sk_load_* helpers also use %r10 and %r9d.
- * See bpf_jit.S
- */
- if (seen_ax_reg)
- /* r10 = skb->data, mov %r10, off32(%rbx) */
- EMIT3_off32(0x4c, 0x8b, 0x93,
- offsetof(struct sk_buff, data));
- EMIT1_off32(0xE8, jmp_offset); /* call */
- break;
-
- case BPF_LD | BPF_IND | BPF_H:
- func = sk_load_half;
- goto common_load;
- case BPF_LD | BPF_ABS | BPF_H:
- func = CHOOSE_LOAD_FUNC(imm32, sk_load_half);
- goto common_load;
- case BPF_LD | BPF_IND | BPF_B:
- func = sk_load_byte;
- goto common_load;
- case BPF_LD | BPF_ABS | BPF_B:
- func = CHOOSE_LOAD_FUNC(imm32, sk_load_byte);
- goto common_load;
-
case BPF_JMP | BPF_EXIT:
if (seen_exit) {
jmp_offset = ctx->cleanup_addr - addrs[i];
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 03/12] bpf: implement ld_abs/ld_ind in native bpf
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
The main part of this work is to finally allow removal of LD_ABS
and LD_IND from the BPF core by reimplementing them through native
eBPF instead. Both LD_ABS/LD_IND were carried over from cBPF and
keeping them around in native eBPF caused way more trouble than
actually worth it. To just list some of the security issues in
the past:
* fdfaf64e7539 ("x86: bpf_jit: support negative offsets")
* 35607b02dbef ("sparc: bpf_jit: fix loads from negative offsets")
* e0ee9c12157d ("x86: bpf_jit: fix two bugs in eBPF JIT compiler")
* 07aee9439454 ("bpf, sparc: fix usage of wrong reg for load_skb_regs after call")
* 6d59b7dbf72e ("bpf, s390x: do not reload skb pointers in non-skb context")
* 87338c8e2cbb ("bpf, ppc64: do not reload skb pointers in non-skb context")
For programs in native eBPF, LD_ABS/LD_IND are pretty much legacy
these days due to their limitations and more efficient/flexible
alternatives that have been developed over time such as direct
packet access. LD_ABS/LD_IND only cover 1/2/4 byte loads into a
register, the load happens in host endianness and its exception
handling can yield unexpected behavior. The latter is explained
in depth in f6b1b3bf0d5f ("bpf: fix subprog verifier bypass by
div/mod by 0 exception") with similar cases of exceptions we had.
In native eBPF more recent program types will disable LD_ABS/LD_IND
altogether through may_access_skb() in verifier, and given the
limitations in terms of exception handling, it's also disabled
in programs that use BPF to BPF calls.
In terms of cBPF, the LD_ABS/LD_IND is used in networking programs
to access packet data. It is not used in seccomp-BPF but programs
that use it for socket filtering or reuseport for demuxing with
cBPF. This is mostly relevant for applications that have not yet
migrated to native eBPF.
The main complexity and source of bugs in LD_ABS/LD_IND is coming
from their implementation in the various JITs. Most of them keep
the model around from cBPF times by implementing a fastpath written
in asm. They use typically two from the BPF program hidden CPU
registers for caching the skb's headlen (skb->len - skb->data_len)
and skb->data. Throughout the JIT phase this requires to keep track
whether LD_ABS/LD_IND are used and if so, the two registers need
to be recached each time a BPF helper would change the underlying
packet data in native eBPF case. At least in eBPF case, available
CPU registers are rare and the additional exit path out of the
asm written JIT helper makes it also inflexible since not all
parts of the JITer are in control from plain C. A LD_ABS/LD_IND
implementation in eBPF therefore allows to significatnly reduce
the complexity in JITs with comparable performance results for
them, e.g.:
test_bpf tcpdump port 22 tcpdump complex
x64 - before 15 21 10 14 19 18
- after 7 10 10 7 10 15
arm64 - before 40 91 92 40 91 151
- after 51 64 73 51 62 113
For cBPF we now track any usage of LD_ABS/LD_IND in bpf_convert_filter()
and cache the skb's headlen and data in the cBPF prologue. The
BPF_REG_TMP gets remapped from R8 to R2 since it's really just
used as a local temporary variable. This allows to shrink the
image on x86_64 also for seccomp programs slightly since mapping
to %rsi is not an ereg. In callee-saved R8 and R9 we now track
skb data and headlen, respectively. For normal prologue emission
in the JITs this does not add any extra instructions since R8, R9
are pushed to stack in any case from eBPF side. cBPF uses the
convert_bpf_ld_abs() emitter which probes the fast path inline
already and falls back to bpf_skb_load_helper_{8,16,32}() helper
relying on the cached skb data and headlen as well. R8 and R9
never need to be reloaded due to bpf_helper_changes_pkt_data()
since all skb access in cBPF is read-only. Then, for the case
of native eBPF, we use the bpf_gen_ld_abs() emitter, which calls
the bpf_skb_load_helper_{8,16,32}_no_cache() helper unconditionally,
does neither cache skb data and headlen nor has an inlined fast
path. The reason for the latter is that native eBPF does not have
any extra registers available anyway, but even if there were, it
avoids any reload of skb data and headlen in the first place.
Additionally, for the negative offsets, we provide an alternative
bpf_skb_load_bytes_relative() helper in eBPF which operates
similarly as bpf_skb_load_bytes(). Tested myself on x64, arm64,
s390x, from Sandipan on ppc64.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf.h | 2 +
include/linux/filter.h | 4 +-
kernel/bpf/core.c | 96 ++-------------------
kernel/bpf/verifier.c | 24 ++++++
net/core/filter.c | 227 +++++++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 255 insertions(+), 98 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 8ea3f6d..6e37974 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -235,6 +235,8 @@ struct bpf_verifier_ops {
struct bpf_insn_access_aux *info);
int (*gen_prologue)(struct bpf_insn *insn, bool direct_write,
const struct bpf_prog *prog);
+ int (*gen_ld_abs)(const struct bpf_insn *orig,
+ struct bpf_insn *insn_buf);
u32 (*convert_ctx_access)(enum bpf_access_type type,
const struct bpf_insn *src,
struct bpf_insn *dst,
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 64899c0..361e8f9 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -47,7 +47,9 @@ struct xdp_buff;
/* Additional register mappings for converted user programs. */
#define BPF_REG_A BPF_REG_0
#define BPF_REG_X BPF_REG_7
-#define BPF_REG_TMP BPF_REG_8
+#define BPF_REG_TMP BPF_REG_2 /* scratch reg */
+#define BPF_REG_D BPF_REG_8 /* data, callee-saved */
+#define BPF_REG_H BPF_REG_9 /* hlen, callee-saved */
/* Kernel hidden auxiliary/helper register for hardening step.
* Only used by eBPF JITs. It's nothing more than a temporary
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 90feeba..1127552 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -634,23 +634,6 @@ static int bpf_jit_blind_insn(const struct bpf_insn *from,
*to++ = BPF_JMP_REG(from->code, from->dst_reg, BPF_REG_AX, off);
break;
- case BPF_LD | BPF_ABS | BPF_W:
- case BPF_LD | BPF_ABS | BPF_H:
- case BPF_LD | BPF_ABS | BPF_B:
- *to++ = BPF_ALU64_IMM(BPF_MOV, BPF_REG_AX, imm_rnd ^ from->imm);
- *to++ = BPF_ALU64_IMM(BPF_XOR, BPF_REG_AX, imm_rnd);
- *to++ = BPF_LD_IND(from->code, BPF_REG_AX, 0);
- break;
-
- case BPF_LD | BPF_IND | BPF_W:
- case BPF_LD | BPF_IND | BPF_H:
- case BPF_LD | BPF_IND | BPF_B:
- *to++ = BPF_ALU64_IMM(BPF_MOV, BPF_REG_AX, imm_rnd ^ from->imm);
- *to++ = BPF_ALU64_IMM(BPF_XOR, BPF_REG_AX, imm_rnd);
- *to++ = BPF_ALU32_REG(BPF_ADD, BPF_REG_AX, from->src_reg);
- *to++ = BPF_LD_IND(from->code, BPF_REG_AX, 0);
- break;
-
case BPF_LD | BPF_IMM | BPF_DW:
*to++ = BPF_ALU64_IMM(BPF_MOV, BPF_REG_AX, imm_rnd ^ aux[1].imm);
*to++ = BPF_ALU64_IMM(BPF_XOR, BPF_REG_AX, imm_rnd);
@@ -891,14 +874,7 @@ EXPORT_SYMBOL_GPL(__bpf_call_base);
INSN_3(LDX, MEM, W), \
INSN_3(LDX, MEM, DW), \
/* Immediate based. */ \
- INSN_3(LD, IMM, DW), \
- /* Misc (old cBPF carry-over). */ \
- INSN_3(LD, ABS, B), \
- INSN_3(LD, ABS, H), \
- INSN_3(LD, ABS, W), \
- INSN_3(LD, IND, B), \
- INSN_3(LD, IND, H), \
- INSN_3(LD, IND, W)
+ INSN_3(LD, IMM, DW)
bool bpf_opcode_in_insntable(u8 code)
{
@@ -908,6 +884,13 @@ bool bpf_opcode_in_insntable(u8 code)
[0 ... 255] = false,
/* Now overwrite non-defaults ... */
BPF_INSN_MAP(BPF_INSN_2_TBL, BPF_INSN_3_TBL),
+ /* UAPI exposed, but rewritten opcodes. cBPF carry-over. */
+ [BPF_LD | BPF_ABS | BPF_B] = true,
+ [BPF_LD | BPF_ABS | BPF_H] = true,
+ [BPF_LD | BPF_ABS | BPF_W] = true,
+ [BPF_LD | BPF_IND | BPF_B] = true,
+ [BPF_LD | BPF_IND | BPF_H] = true,
+ [BPF_LD | BPF_IND | BPF_W] = true,
};
#undef BPF_INSN_3_TBL
#undef BPF_INSN_2_TBL
@@ -938,8 +921,6 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
#undef BPF_INSN_3_LBL
#undef BPF_INSN_2_LBL
u32 tail_call_cnt = 0;
- void *ptr;
- int off;
#define CONT ({ insn++; goto select_insn; })
#define CONT_JMP ({ insn++; goto select_insn; })
@@ -1266,67 +1247,6 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
atomic64_add((u64) SRC, (atomic64_t *)(unsigned long)
(DST + insn->off));
CONT;
- LD_ABS_W: /* BPF_R0 = ntohl(*(u32 *) (skb->data + imm32)) */
- off = IMM;
-load_word:
- /* BPF_LD + BPD_ABS and BPF_LD + BPF_IND insns are only
- * appearing in the programs where ctx == skb
- * (see may_access_skb() in the verifier). All programs
- * keep 'ctx' in regs[BPF_REG_CTX] == BPF_R6,
- * bpf_convert_filter() saves it in BPF_R6, internal BPF
- * verifier will check that BPF_R6 == ctx.
- *
- * BPF_ABS and BPF_IND are wrappers of function calls,
- * so they scratch BPF_R1-BPF_R5 registers, preserve
- * BPF_R6-BPF_R9, and store return value into BPF_R0.
- *
- * Implicit input:
- * ctx == skb == BPF_R6 == CTX
- *
- * Explicit input:
- * SRC == any register
- * IMM == 32-bit immediate
- *
- * Output:
- * BPF_R0 - 8/16/32-bit skb data converted to cpu endianness
- */
-
- ptr = bpf_load_pointer((struct sk_buff *) (unsigned long) CTX, off, 4, &tmp);
- if (likely(ptr != NULL)) {
- BPF_R0 = get_unaligned_be32(ptr);
- CONT;
- }
-
- return 0;
- LD_ABS_H: /* BPF_R0 = ntohs(*(u16 *) (skb->data + imm32)) */
- off = IMM;
-load_half:
- ptr = bpf_load_pointer((struct sk_buff *) (unsigned long) CTX, off, 2, &tmp);
- if (likely(ptr != NULL)) {
- BPF_R0 = get_unaligned_be16(ptr);
- CONT;
- }
-
- return 0;
- LD_ABS_B: /* BPF_R0 = *(u8 *) (skb->data + imm32) */
- off = IMM;
-load_byte:
- ptr = bpf_load_pointer((struct sk_buff *) (unsigned long) CTX, off, 1, &tmp);
- if (likely(ptr != NULL)) {
- BPF_R0 = *(u8 *)ptr;
- CONT;
- }
-
- return 0;
- LD_IND_W: /* BPF_R0 = ntohl(*(u32 *) (skb->data + src_reg + imm32)) */
- off = IMM + SRC;
- goto load_word;
- LD_IND_H: /* BPF_R0 = ntohs(*(u16 *) (skb->data + src_reg + imm32)) */
- off = IMM + SRC;
- goto load_half;
- LD_IND_B: /* BPF_R0 = *(u8 *) (skb->data + src_reg + imm32) */
- off = IMM + SRC;
- goto load_byte;
default_label:
/* If we ever reach this, we have a bug somewhere. Die hard here
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 712d865..324af92 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3880,6 +3880,11 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
return -EINVAL;
}
+ if (!env->ops->gen_ld_abs) {
+ verbose(env, "bpf verifier is misconfigured\n");
+ return -EINVAL;
+ }
+
if (env->subprog_cnt) {
/* when program has LD_ABS insn JITs and interpreter assume
* that r1 == ctx == skb which is not the case for callees
@@ -5515,6 +5520,25 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
continue;
}
+ if (BPF_CLASS(insn->code) == BPF_LD &&
+ (BPF_MODE(insn->code) == BPF_ABS ||
+ BPF_MODE(insn->code) == BPF_IND)) {
+ cnt = env->ops->gen_ld_abs(insn, insn_buf);
+ if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf)) {
+ verbose(env, "bpf verifier is misconfigured\n");
+ return -EINVAL;
+ }
+
+ new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
+ if (!new_prog)
+ return -ENOMEM;
+
+ delta += cnt - 1;
+ env->prog = prog = new_prog;
+ insn = new_prog->insnsi + i + delta;
+ continue;
+ }
+
if (insn->code != (BPF_JMP | BPF_CALL))
continue;
if (insn->src_reg == BPF_PSEUDO_CALL)
diff --git a/net/core/filter.c b/net/core/filter.c
index d2de6b8..3159f53 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -161,6 +161,87 @@ BPF_CALL_3(bpf_skb_get_nlattr_nest, struct sk_buff *, skb, u32, a, u32, x)
return 0;
}
+BPF_CALL_4(bpf_skb_load_helper_8, const struct sk_buff *, skb, const void *,
+ data, int, headlen, int, offset)
+{
+ u8 tmp, *ptr;
+ const int len = sizeof(tmp);
+
+ if (offset >= 0) {
+ if (headlen - offset >= len)
+ return *(u8 *)(data + offset);
+ if (!skb_copy_bits(skb, offset, &tmp, sizeof(tmp)))
+ return tmp;
+ } else {
+ ptr = bpf_internal_load_pointer_neg_helper(skb, offset, len);
+ if (likely(ptr))
+ return *(u8 *)ptr;
+ }
+
+ return -EFAULT;
+}
+
+BPF_CALL_2(bpf_skb_load_helper_8_no_cache, const struct sk_buff *, skb,
+ int, offset)
+{
+ return ____bpf_skb_load_helper_8(skb, skb->data, skb->len - skb->data_len,
+ offset);
+}
+
+BPF_CALL_4(bpf_skb_load_helper_16, const struct sk_buff *, skb, const void *,
+ data, int, headlen, int, offset)
+{
+ u16 tmp, *ptr;
+ const int len = sizeof(tmp);
+
+ if (offset >= 0) {
+ if (headlen - offset >= len)
+ return get_unaligned_be16(data + offset);
+ if (!skb_copy_bits(skb, offset, &tmp, sizeof(tmp)))
+ return be16_to_cpu(tmp);
+ } else {
+ ptr = bpf_internal_load_pointer_neg_helper(skb, offset, len);
+ if (likely(ptr))
+ return get_unaligned_be16(ptr);
+ }
+
+ return -EFAULT;
+}
+
+BPF_CALL_2(bpf_skb_load_helper_16_no_cache, const struct sk_buff *, skb,
+ int, offset)
+{
+ return ____bpf_skb_load_helper_16(skb, skb->data, skb->len - skb->data_len,
+ offset);
+}
+
+BPF_CALL_4(bpf_skb_load_helper_32, const struct sk_buff *, skb, const void *,
+ data, int, headlen, int, offset)
+{
+ u32 tmp, *ptr;
+ const int len = sizeof(tmp);
+
+ if (likely(offset >= 0)) {
+ if (headlen - offset >= len)
+ return get_unaligned_be32(data + offset);
+ if (!skb_copy_bits(skb, offset, &tmp, sizeof(tmp)))
+ return be32_to_cpu(tmp);
+ } else {
+ ptr = bpf_internal_load_pointer_neg_helper(skb, offset, len);
+ if (likely(ptr))
+ return get_unaligned_be32(ptr);
+ }
+
+ return -EFAULT;
+}
+
+BPF_CALL_2(bpf_skb_load_helper_32_no_cache, const struct sk_buff *, skb,
+ int, offset)
+{
+ return ____bpf_skb_load_helper_32(skb, skb->data, skb->len - skb->data_len,
+ offset);
+}
+
BPF_CALL_0(bpf_get_raw_cpu_id)
{
return raw_smp_processor_id();
@@ -353,26 +434,87 @@ static bool convert_bpf_extensions(struct sock_filter *fp,
return true;
}
+static bool convert_bpf_ld_abs(struct sock_filter *fp, struct bpf_insn **insnp)
+{
+ const bool unaligned_ok = IS_BUILTIN(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS);
+ int size = bpf_size_to_bytes(BPF_SIZE(fp->code));
+ bool endian = BPF_SIZE(fp->code) == BPF_H ||
+ BPF_SIZE(fp->code) == BPF_W;
+ bool indirect = BPF_MODE(fp->code) == BPF_IND;
+ const int ip_align = NET_IP_ALIGN;
+ struct bpf_insn *insn = *insnp;
+ int offset = fp->k;
+
+ if (!indirect &&
+ ((unaligned_ok && offset >= 0) ||
+ (!unaligned_ok && offset >= 0 &&
+ offset + ip_align >= 0 &&
+ offset + ip_align % size == 0))) {
+ *insn++ = BPF_MOV64_REG(BPF_REG_TMP, BPF_REG_H);
+ *insn++ = BPF_ALU64_IMM(BPF_SUB, BPF_REG_TMP, offset);
+ *insn++ = BPF_JMP_IMM(BPF_JSLT, BPF_REG_TMP, size, 2 + endian);
+ *insn++ = BPF_LDX_MEM(BPF_SIZE(fp->code), BPF_REG_A, BPF_REG_D,
+ offset);
+ if (endian)
+ *insn++ = BPF_ENDIAN(BPF_FROM_BE, BPF_REG_A, size * 8);
+ *insn++ = BPF_JMP_A(8);
+ }
+
+ *insn++ = BPF_MOV64_REG(BPF_REG_ARG1, BPF_REG_CTX);
+ *insn++ = BPF_MOV64_REG(BPF_REG_ARG2, BPF_REG_D);
+ *insn++ = BPF_MOV64_REG(BPF_REG_ARG3, BPF_REG_H);
+ if (!indirect) {
+ *insn++ = BPF_MOV64_IMM(BPF_REG_ARG4, offset);
+ } else {
+ *insn++ = BPF_MOV64_REG(BPF_REG_ARG4, BPF_REG_X);
+ if (fp->k)
+ *insn++ = BPF_ALU64_IMM(BPF_ADD, BPF_REG_ARG4, offset);
+ }
+
+ switch (BPF_SIZE(fp->code)) {
+ case BPF_B:
+ *insn++ = BPF_EMIT_CALL(bpf_skb_load_helper_8);
+ break;
+ case BPF_H:
+ *insn++ = BPF_EMIT_CALL(bpf_skb_load_helper_16);
+ break;
+ case BPF_W:
+ *insn++ = BPF_EMIT_CALL(bpf_skb_load_helper_32);
+ break;
+ default:
+ return false;
+ }
+
+ *insn++ = BPF_JMP_IMM(BPF_JSGE, BPF_REG_A, 0, 2);
+ *insn++ = BPF_ALU32_REG(BPF_XOR, BPF_REG_A, BPF_REG_A);
+ *insn = BPF_EXIT_INSN();
+
+ *insnp = insn;
+ return true;
+}
+
/**
* bpf_convert_filter - convert filter program
* @prog: the user passed filter program
* @len: the length of the user passed filter program
* @new_prog: allocated 'struct bpf_prog' or NULL
* @new_len: pointer to store length of converted program
+ * @seen_ld_abs: bool whether we've seen ld_abs/ind
*
* Remap 'sock_filter' style classic BPF (cBPF) instruction set to 'bpf_insn'
* style extended BPF (eBPF).
* Conversion workflow:
*
* 1) First pass for calculating the new program length:
- * bpf_convert_filter(old_prog, old_len, NULL, &new_len)
+ * bpf_convert_filter(old_prog, old_len, NULL, &new_len, &seen_ld_abs)
*
* 2) 2nd pass to remap in two passes: 1st pass finds new
* jump offsets, 2nd pass remapping:
- * bpf_convert_filter(old_prog, old_len, new_prog, &new_len);
+ * bpf_convert_filter(old_prog, old_len, new_prog, &new_len, &seen_ld_abs)
*/
static int bpf_convert_filter(struct sock_filter *prog, int len,
- struct bpf_prog *new_prog, int *new_len)
+ struct bpf_prog *new_prog, int *new_len,
+ bool *seen_ld_abs)
{
int new_flen = 0, pass = 0, target, i, stack_off;
struct bpf_insn *new_insn, *first_insn = NULL;
@@ -411,12 +553,27 @@ static int bpf_convert_filter(struct sock_filter *prog, int len,
* do this ourself. Initial CTX is present in BPF_REG_ARG1.
*/
*new_insn++ = BPF_MOV64_REG(BPF_REG_CTX, BPF_REG_ARG1);
+ if (*seen_ld_abs) {
+ /* For packet access in classic BPF, cache skb->data
+ * in callee-saved BPF R8 and skb->len - skb->data_len
+ * (headlen) in BPF R9. Since classic BPF is read-only
+ * on CTX, we only need to cache it once.
+ */
+ *new_insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct sk_buff, data),
+ BPF_REG_D, BPF_REG_CTX,
+ offsetof(struct sk_buff, data));
+ *new_insn++ = BPF_LDX_MEM(BPF_W, BPF_REG_H, BPF_REG_CTX,
+ offsetof(struct sk_buff, len));
+ *new_insn++ = BPF_LDX_MEM(BPF_W, BPF_REG_TMP, BPF_REG_CTX,
+ offsetof(struct sk_buff, data_len));
+ *new_insn++ = BPF_ALU32_REG(BPF_SUB, BPF_REG_H, BPF_REG_TMP);
+ }
} else {
new_insn += 3;
}
for (i = 0; i < len; fp++, i++) {
- struct bpf_insn tmp_insns[6] = { };
+ struct bpf_insn tmp_insns[32] = { };
struct bpf_insn *insn = tmp_insns;
if (addrs)
@@ -459,6 +616,11 @@ static int bpf_convert_filter(struct sock_filter *prog, int len,
BPF_MODE(fp->code) == BPF_ABS &&
convert_bpf_extensions(fp, &insn))
break;
+ if (BPF_CLASS(fp->code) == BPF_LD &&
+ convert_bpf_ld_abs(fp, &insn)) {
+ *seen_ld_abs = true;
+ break;
+ }
if (fp->code == (BPF_ALU | BPF_DIV | BPF_X) ||
fp->code == (BPF_ALU | BPF_MOD | BPF_X)) {
@@ -561,11 +723,16 @@ static int bpf_convert_filter(struct sock_filter *prog, int len,
break;
/* ldxb 4 * ([14] & 0xf) is remaped into 6 insns. */
- case BPF_LDX | BPF_MSH | BPF_B:
+ case BPF_LDX | BPF_MSH | BPF_B: {
+ struct sock_filter tmp = {
+ .code = BPF_LD | BPF_ABS | BPF_B,
+ .k = fp->k,
+ };
/* tmp = A */
*insn++ = BPF_MOV64_REG(BPF_REG_TMP, BPF_REG_A);
/* A = BPF_R0 = *(u8 *) (skb->data + K) */
- *insn++ = BPF_LD_ABS(BPF_B, fp->k);
+ convert_bpf_ld_abs(&tmp, &insn);
+ insn++;
/* A &= 0xf */
*insn++ = BPF_ALU32_IMM(BPF_AND, BPF_REG_A, 0xf);
/* A <<= 2 */
@@ -575,7 +742,7 @@ static int bpf_convert_filter(struct sock_filter *prog, int len,
/* A = tmp */
*insn = BPF_MOV64_REG(BPF_REG_A, BPF_REG_TMP);
break;
-
+ }
/* RET_K is remaped into 2 insns. RET_A case doesn't need an
* extra mov as BPF_REG_0 is already mapped into BPF_REG_A.
*/
@@ -657,6 +824,8 @@ static int bpf_convert_filter(struct sock_filter *prog, int len,
if (!new_prog) {
/* Only calculating new length. */
*new_len = new_insn - first_insn;
+ if (*seen_ld_abs)
+ *new_len += 4; /* Prologue bits. */
return 0;
}
@@ -1018,6 +1187,7 @@ static struct bpf_prog *bpf_migrate_filter(struct bpf_prog *fp)
struct sock_filter *old_prog;
struct bpf_prog *old_fp;
int err, new_len, old_len = fp->len;
+ bool seen_ld_abs = false;
/* We are free to overwrite insns et al right here as it
* won't be used at this point in time anymore internally
@@ -1039,7 +1209,8 @@ static struct bpf_prog *bpf_migrate_filter(struct bpf_prog *fp)
}
/* 1st pass: calculate the new program length. */
- err = bpf_convert_filter(old_prog, old_len, NULL, &new_len);
+ err = bpf_convert_filter(old_prog, old_len, NULL, &new_len,
+ &seen_ld_abs);
if (err)
goto out_err_free;
@@ -1058,7 +1229,8 @@ static struct bpf_prog *bpf_migrate_filter(struct bpf_prog *fp)
fp->len = new_len;
/* 2nd pass: remap sock_filter insns into bpf_insn insns. */
- err = bpf_convert_filter(old_prog, old_len, fp, &new_len);
+ err = bpf_convert_filter(old_prog, old_len, fp, &new_len,
+ &seen_ld_abs);
if (err)
/* 2nd bpf_convert_filter() can fail only if it fails
* to allocate memory, remapping must succeed. Note,
@@ -4302,6 +4474,41 @@ static int bpf_unclone_prologue(struct bpf_insn *insn_buf, bool direct_write,
return insn - insn_buf;
}
+static int bpf_gen_ld_abs(const struct bpf_insn *orig,
+ struct bpf_insn *insn_buf)
+{
+ bool indirect = BPF_MODE(orig->code) == BPF_IND;
+ struct bpf_insn *insn = insn_buf;
+
+ /* We're guaranteed here that CTX is in R6. */
+ *insn++ = BPF_MOV64_REG(BPF_REG_1, BPF_REG_CTX);
+ if (!indirect) {
+ *insn++ = BPF_MOV64_IMM(BPF_REG_2, orig->imm);
+ } else {
+ *insn++ = BPF_MOV64_REG(BPF_REG_2, orig->src_reg);
+ if (orig->imm)
+ *insn++ = BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, orig->imm);
+ }
+
+ switch (BPF_SIZE(orig->code)) {
+ case BPF_B:
+ *insn++ = BPF_EMIT_CALL(bpf_skb_load_helper_8_no_cache);
+ break;
+ case BPF_H:
+ *insn++ = BPF_EMIT_CALL(bpf_skb_load_helper_16_no_cache);
+ break;
+ case BPF_W:
+ *insn++ = BPF_EMIT_CALL(bpf_skb_load_helper_32_no_cache);
+ break;
+ }
+
+ *insn++ = BPF_JMP_IMM(BPF_JSGE, BPF_REG_0, 0, 2);
+ *insn++ = BPF_ALU32_REG(BPF_XOR, BPF_REG_0, BPF_REG_0);
+ *insn++ = BPF_EXIT_INSN();
+
+ return insn - insn_buf;
+}
+
static int tc_cls_act_prologue(struct bpf_insn *insn_buf, bool direct_write,
const struct bpf_prog *prog)
{
@@ -5571,6 +5778,7 @@ const struct bpf_verifier_ops sk_filter_verifier_ops = {
.get_func_proto = sk_filter_func_proto,
.is_valid_access = sk_filter_is_valid_access,
.convert_ctx_access = bpf_convert_ctx_access,
+ .gen_ld_abs = bpf_gen_ld_abs,
};
const struct bpf_prog_ops sk_filter_prog_ops = {
@@ -5582,6 +5790,7 @@ const struct bpf_verifier_ops tc_cls_act_verifier_ops = {
.is_valid_access = tc_cls_act_is_valid_access,
.convert_ctx_access = tc_cls_act_convert_ctx_access,
.gen_prologue = tc_cls_act_prologue,
+ .gen_ld_abs = bpf_gen_ld_abs,
};
const struct bpf_prog_ops tc_cls_act_prog_ops = {
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 04/12] bpf: add skb_load_bytes_relative helper
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
This adds a small BPF helper similar to bpf_skb_load_bytes() that
is able to load relative to mac/net header offset from the skb's
linear data. Compared to bpf_skb_load_bytes(), it takes a fith
argument namely start_header, which is either BPF_HDR_START_MAC
or BPF_HDR_START_NET. This allows for a more flexible alternative
compared to LD_ABS/LD_IND with negative offset. It's enabled for
tc BPF programs as well as sock filter program types where it's
mainly useful in reuseport programs to ease access to lower header
data.
Reference: https://lists.iovisor.org/pipermail/iovisor-dev/2017-March/000698.html
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
include/uapi/linux/bpf.h | 33 ++++++++++++++++++++++++++++++++-
net/core/filter.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 8daef73..83a95ae 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1801,6 +1801,30 @@ union bpf_attr {
* Return
* a non-negative value equal to or less than size on success, or
* a negative error in case of failure.
+ *
+ * int skb_load_bytes_relative(const struct sk_buff *skb, u32 offset, void *to, u32 len, u32 start_header)
+ * Description
+ * This helper is similar to **bpf_skb_load_bytes**\ () in that
+ * it provides an easy way to load *len* bytes from *offset*
+ * from the packet associated to *skb*, into the buffer pointed
+ * by *to*. The difference to **bpf_skb_load_bytes**\ () is that
+ * a fifth argument *start_header* exists in order to select a
+ * base offset to start from. *start_header* can be one of:
+ *
+ * **BPF_HDR_START_MAC**
+ * Base offset to load data from is *skb*'s mac header.
+ * **BPF_HDR_START_NET**
+ * Base offset to load data from is *skb*'s network header.
+ *
+ * In general, "direct packet access" is the preferred method to
+ * access packet data, however, this helper is in particular useful
+ * in socket filters where *skb*\ **->data** does not always point
+ * to the start of the mac header and where "direct packet access"
+ * is not available.
+ *
+ * Return
+ * 0 on success, or a negative error in case of failure.
+ *
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -1870,7 +1894,8 @@ union bpf_attr {
FN(bind), \
FN(xdp_adjust_tail), \
FN(skb_get_xfrm_state), \
- FN(get_stack),
+ FN(get_stack), \
+ FN(skb_load_bytes_relative),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -1931,6 +1956,12 @@ enum bpf_adj_room_mode {
BPF_ADJ_ROOM_NET,
};
+/* Mode for BPF_FUNC_skb_load_bytes_relative helper. */
+enum bpf_hdr_start_off {
+ BPF_HDR_START_MAC,
+ BPF_HDR_START_NET,
+};
+
/* user accessible mirror of in-kernel sk_buff.
* new fields can only be added to the end of this structure
*/
diff --git a/net/core/filter.c b/net/core/filter.c
index 3159f53..516ac1b 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1678,6 +1678,47 @@ static const struct bpf_func_proto bpf_skb_load_bytes_proto = {
.arg4_type = ARG_CONST_SIZE,
};
+BPF_CALL_5(bpf_skb_load_bytes_relative, const struct sk_buff *, skb,
+ u32, offset, void *, to, u32, len, u32, start_header)
+{
+ u8 *ptr;
+
+ if (unlikely(offset > 0xffff || len > skb_headlen(skb)))
+ goto err_clear;
+
+ switch (start_header) {
+ case BPF_HDR_START_MAC:
+ ptr = skb_mac_header(skb) + offset;
+ break;
+ case BPF_HDR_START_NET:
+ ptr = skb_network_header(skb) + offset;
+ break;
+ default:
+ goto err_clear;
+ }
+
+ if (likely(ptr >= skb_mac_header(skb) &&
+ ptr + len <= skb_tail_pointer(skb))) {
+ memcpy(to, ptr, len);
+ return 0;
+ }
+
+err_clear:
+ memset(to, 0, len);
+ return -EFAULT;
+}
+
+static const struct bpf_func_proto bpf_skb_load_bytes_relative_proto = {
+ .func = bpf_skb_load_bytes_relative,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_PTR_TO_UNINIT_MEM,
+ .arg4_type = ARG_CONST_SIZE,
+ .arg5_type = ARG_ANYTHING,
+};
+
BPF_CALL_2(bpf_skb_pull_data, struct sk_buff *, skb, u32, len)
{
/* Idea is the following: should the needed direct read/write
@@ -4028,6 +4069,8 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
switch (func_id) {
case BPF_FUNC_skb_load_bytes:
return &bpf_skb_load_bytes_proto;
+ case BPF_FUNC_skb_load_bytes_relative:
+ return &bpf_skb_load_bytes_relative_proto;
case BPF_FUNC_get_socket_cookie:
return &bpf_get_socket_cookie_proto;
case BPF_FUNC_get_socket_uid:
@@ -4045,6 +4088,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_skb_store_bytes_proto;
case BPF_FUNC_skb_load_bytes:
return &bpf_skb_load_bytes_proto;
+ case BPF_FUNC_skb_load_bytes_relative:
+ return &bpf_skb_load_bytes_relative_proto;
case BPF_FUNC_skb_pull_data:
return &bpf_skb_pull_data_proto;
case BPF_FUNC_csum_diff:
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 02/12] bpf: migrate ebpf ld_abs/ld_ind tests to test_verifier
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
Remove all eBPF tests involving LD_ABS/LD_IND from test_bpf.ko. Reason
is that the eBPF tests from test_bpf module do not go via BPF verifier
and therefore any instruction rewrites from verifier cannot take place.
Therefore, move them into test_verifier which runs out of user space,
so that verfier can rewrite LD_ABS/LD_IND internally in upcoming patches.
It will have the same effect since runtime tests are also performed from
there. This also allows to finally unexport bpf_skb_vlan_{push,pop}_proto
and keep it internal to core kernel.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf.h | 2 -
lib/test_bpf.c | 212 ----------------------
net/core/filter.c | 6 +-
tools/testing/selftests/bpf/test_verifier.c | 266 +++++++++++++++++++++++++++-
4 files changed, 261 insertions(+), 225 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index c553f6f..8ea3f6d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -689,8 +689,6 @@ extern const struct bpf_func_proto bpf_ktime_get_ns_proto;
extern const struct bpf_func_proto bpf_get_current_pid_tgid_proto;
extern const struct bpf_func_proto bpf_get_current_uid_gid_proto;
extern const struct bpf_func_proto bpf_get_current_comm_proto;
-extern const struct bpf_func_proto bpf_skb_vlan_push_proto;
-extern const struct bpf_func_proto bpf_skb_vlan_pop_proto;
extern const struct bpf_func_proto bpf_get_stackid_proto;
extern const struct bpf_func_proto bpf_get_stack_proto;
extern const struct bpf_func_proto bpf_sock_map_update_proto;
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 8e15780..35f49bd 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -386,116 +386,6 @@ static int bpf_fill_ld_abs_get_processor_id(struct bpf_test *self)
return 0;
}
-#define PUSH_CNT 68
-/* test: {skb->data[0], vlan_push} x 68 + {skb->data[0], vlan_pop} x 68 */
-static int bpf_fill_ld_abs_vlan_push_pop(struct bpf_test *self)
-{
- unsigned int len = BPF_MAXINSNS;
- struct bpf_insn *insn;
- int i = 0, j, k = 0;
-
- insn = kmalloc_array(len, sizeof(*insn), GFP_KERNEL);
- if (!insn)
- return -ENOMEM;
-
- insn[i++] = BPF_MOV64_REG(R6, R1);
-loop:
- for (j = 0; j < PUSH_CNT; j++) {
- insn[i++] = BPF_LD_ABS(BPF_B, 0);
- insn[i] = BPF_JMP_IMM(BPF_JNE, R0, 0x34, len - i - 2);
- i++;
- insn[i++] = BPF_MOV64_REG(R1, R6);
- insn[i++] = BPF_MOV64_IMM(R2, 1);
- insn[i++] = BPF_MOV64_IMM(R3, 2);
- insn[i++] = BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
- bpf_skb_vlan_push_proto.func - __bpf_call_base);
- insn[i] = BPF_JMP_IMM(BPF_JNE, R0, 0, len - i - 2);
- i++;
- }
-
- for (j = 0; j < PUSH_CNT; j++) {
- insn[i++] = BPF_LD_ABS(BPF_B, 0);
- insn[i] = BPF_JMP_IMM(BPF_JNE, R0, 0x34, len - i - 2);
- i++;
- insn[i++] = BPF_MOV64_REG(R1, R6);
- insn[i++] = BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
- bpf_skb_vlan_pop_proto.func - __bpf_call_base);
- insn[i] = BPF_JMP_IMM(BPF_JNE, R0, 0, len - i - 2);
- i++;
- }
- if (++k < 5)
- goto loop;
-
- for (; i < len - 1; i++)
- insn[i] = BPF_ALU32_IMM(BPF_MOV, R0, 0xbef);
-
- insn[len - 1] = BPF_EXIT_INSN();
-
- self->u.ptr.insns = insn;
- self->u.ptr.len = len;
-
- return 0;
-}
-
-static int bpf_fill_ld_abs_vlan_push_pop2(struct bpf_test *self)
-{
- struct bpf_insn *insn;
-
- insn = kmalloc_array(16, sizeof(*insn), GFP_KERNEL);
- if (!insn)
- return -ENOMEM;
-
- /* Due to func address being non-const, we need to
- * assemble this here.
- */
- insn[0] = BPF_MOV64_REG(R6, R1);
- insn[1] = BPF_LD_ABS(BPF_B, 0);
- insn[2] = BPF_LD_ABS(BPF_H, 0);
- insn[3] = BPF_LD_ABS(BPF_W, 0);
- insn[4] = BPF_MOV64_REG(R7, R6);
- insn[5] = BPF_MOV64_IMM(R6, 0);
- insn[6] = BPF_MOV64_REG(R1, R7);
- insn[7] = BPF_MOV64_IMM(R2, 1);
- insn[8] = BPF_MOV64_IMM(R3, 2);
- insn[9] = BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
- bpf_skb_vlan_push_proto.func - __bpf_call_base);
- insn[10] = BPF_MOV64_REG(R6, R7);
- insn[11] = BPF_LD_ABS(BPF_B, 0);
- insn[12] = BPF_LD_ABS(BPF_H, 0);
- insn[13] = BPF_LD_ABS(BPF_W, 0);
- insn[14] = BPF_MOV64_IMM(R0, 42);
- insn[15] = BPF_EXIT_INSN();
-
- self->u.ptr.insns = insn;
- self->u.ptr.len = 16;
-
- return 0;
-}
-
-static int bpf_fill_jump_around_ld_abs(struct bpf_test *self)
-{
- unsigned int len = BPF_MAXINSNS;
- struct bpf_insn *insn;
- int i = 0;
-
- insn = kmalloc_array(len, sizeof(*insn), GFP_KERNEL);
- if (!insn)
- return -ENOMEM;
-
- insn[i++] = BPF_MOV64_REG(R6, R1);
- insn[i++] = BPF_LD_ABS(BPF_B, 0);
- insn[i] = BPF_JMP_IMM(BPF_JEQ, R0, 10, len - i - 2);
- i++;
- while (i < len - 1)
- insn[i++] = BPF_LD_ABS(BPF_B, 1);
- insn[i] = BPF_EXIT_INSN();
-
- self->u.ptr.insns = insn;
- self->u.ptr.len = len;
-
- return 0;
-}
-
static int __bpf_fill_stxdw(struct bpf_test *self, int size)
{
unsigned int len = BPF_MAXINSNS;
@@ -1988,40 +1878,6 @@ static struct bpf_test tests[] = {
{ { 0, -1 } }
},
{
- "INT: DIV + ABS",
- .u.insns_int = {
- BPF_ALU64_REG(BPF_MOV, R6, R1),
- BPF_LD_ABS(BPF_B, 3),
- BPF_ALU64_IMM(BPF_MOV, R2, 2),
- BPF_ALU32_REG(BPF_DIV, R0, R2),
- BPF_ALU64_REG(BPF_MOV, R8, R0),
- BPF_LD_ABS(BPF_B, 4),
- BPF_ALU64_REG(BPF_ADD, R8, R0),
- BPF_LD_IND(BPF_B, R8, -70),
- BPF_EXIT_INSN(),
- },
- INTERNAL,
- { 10, 20, 30, 40, 50 },
- { { 4, 0 }, { 5, 10 } }
- },
- {
- /* This one doesn't go through verifier, but is just raw insn
- * as opposed to cBPF tests from here. Thus div by 0 tests are
- * done in test_verifier in BPF kselftests.
- */
- "INT: DIV by -1",
- .u.insns_int = {
- BPF_ALU64_REG(BPF_MOV, R6, R1),
- BPF_ALU64_IMM(BPF_MOV, R7, -1),
- BPF_LD_ABS(BPF_B, 3),
- BPF_ALU32_REG(BPF_DIV, R0, R7),
- BPF_EXIT_INSN(),
- },
- INTERNAL,
- { 10, 20, 30, 40, 50 },
- { { 3, 0 }, { 4, 0 } }
- },
- {
"check: missing ret",
.u.insns = {
BPF_STMT(BPF_LD | BPF_IMM, 1),
@@ -2383,50 +2239,6 @@ static struct bpf_test tests[] = {
{ },
{ { 0, 1 } }
},
- {
- "nmap reduced",
- .u.insns_int = {
- BPF_MOV64_REG(R6, R1),
- BPF_LD_ABS(BPF_H, 12),
- BPF_JMP_IMM(BPF_JNE, R0, 0x806, 28),
- BPF_LD_ABS(BPF_H, 12),
- BPF_JMP_IMM(BPF_JNE, R0, 0x806, 26),
- BPF_MOV32_IMM(R0, 18),
- BPF_STX_MEM(BPF_W, R10, R0, -64),
- BPF_LDX_MEM(BPF_W, R7, R10, -64),
- BPF_LD_IND(BPF_W, R7, 14),
- BPF_STX_MEM(BPF_W, R10, R0, -60),
- BPF_MOV32_IMM(R0, 280971478),
- BPF_STX_MEM(BPF_W, R10, R0, -56),
- BPF_LDX_MEM(BPF_W, R7, R10, -56),
- BPF_LDX_MEM(BPF_W, R0, R10, -60),
- BPF_ALU32_REG(BPF_SUB, R0, R7),
- BPF_JMP_IMM(BPF_JNE, R0, 0, 15),
- BPF_LD_ABS(BPF_H, 12),
- BPF_JMP_IMM(BPF_JNE, R0, 0x806, 13),
- BPF_MOV32_IMM(R0, 22),
- BPF_STX_MEM(BPF_W, R10, R0, -56),
- BPF_LDX_MEM(BPF_W, R7, R10, -56),
- BPF_LD_IND(BPF_H, R7, 14),
- BPF_STX_MEM(BPF_W, R10, R0, -52),
- BPF_MOV32_IMM(R0, 17366),
- BPF_STX_MEM(BPF_W, R10, R0, -48),
- BPF_LDX_MEM(BPF_W, R7, R10, -48),
- BPF_LDX_MEM(BPF_W, R0, R10, -52),
- BPF_ALU32_REG(BPF_SUB, R0, R7),
- BPF_JMP_IMM(BPF_JNE, R0, 0, 2),
- BPF_MOV32_IMM(R0, 256),
- BPF_EXIT_INSN(),
- BPF_MOV32_IMM(R0, 0),
- BPF_EXIT_INSN(),
- },
- INTERNAL,
- { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x08, 0x06, 0, 0,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 0x10, 0xbf, 0x48, 0xd6, 0x43, 0xd6},
- { { 38, 256 } },
- .stack_depth = 64,
- },
/* BPF_ALU | BPF_MOV | BPF_X */
{
"ALU_MOV_X: dst = 2",
@@ -5485,22 +5297,6 @@ static struct bpf_test tests[] = {
{ { 1, 0xbee } },
.fill_helper = bpf_fill_ld_abs_get_processor_id,
},
- {
- "BPF_MAXINSNS: ld_abs+vlan_push/pop",
- { },
- INTERNAL,
- { 0x34 },
- { { ETH_HLEN, 0xbef } },
- .fill_helper = bpf_fill_ld_abs_vlan_push_pop,
- },
- {
- "BPF_MAXINSNS: jump around ld_abs",
- { },
- INTERNAL,
- { 10, 11 },
- { { 2, 10 } },
- .fill_helper = bpf_fill_jump_around_ld_abs,
- },
/*
* LD_IND / LD_ABS on fragmented SKBs
*/
@@ -6127,14 +5923,6 @@ static struct bpf_test tests[] = {
{},
{ {0x1, 0x42 } },
},
- {
- "LD_ABS with helper changing skb data",
- { },
- INTERNAL,
- { 0x34 },
- { { ETH_HLEN, 42 } },
- .fill_helper = bpf_fill_ld_abs_vlan_push_pop2,
- },
/* Checking interpreter vs JIT wrt signed extended imms. */
{
"JNE signed compare, test 1",
diff --git a/net/core/filter.c b/net/core/filter.c
index 07fe378..d2de6b8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2180,7 +2180,7 @@ BPF_CALL_3(bpf_skb_vlan_push, struct sk_buff *, skb, __be16, vlan_proto,
return ret;
}
-const struct bpf_func_proto bpf_skb_vlan_push_proto = {
+static const struct bpf_func_proto bpf_skb_vlan_push_proto = {
.func = bpf_skb_vlan_push,
.gpl_only = false,
.ret_type = RET_INTEGER,
@@ -2188,7 +2188,6 @@ const struct bpf_func_proto bpf_skb_vlan_push_proto = {
.arg2_type = ARG_ANYTHING,
.arg3_type = ARG_ANYTHING,
};
-EXPORT_SYMBOL_GPL(bpf_skb_vlan_push_proto);
BPF_CALL_1(bpf_skb_vlan_pop, struct sk_buff *, skb)
{
@@ -2202,13 +2201,12 @@ BPF_CALL_1(bpf_skb_vlan_pop, struct sk_buff *, skb)
return ret;
}
-const struct bpf_func_proto bpf_skb_vlan_pop_proto = {
+static const struct bpf_func_proto bpf_skb_vlan_pop_proto = {
.func = bpf_skb_vlan_pop,
.gpl_only = false,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
};
-EXPORT_SYMBOL_GPL(bpf_skb_vlan_pop_proto);
static int bpf_skb_generic_push(struct sk_buff *skb, u32 off, u32 len)
{
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 1acafe26..275b457 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -47,7 +47,7 @@
# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
#endif
-#define MAX_INSNS 512
+#define MAX_INSNS BPF_MAXINSNS
#define MAX_FIXUPS 8
#define MAX_NR_MAPS 4
#define POINTER_VALUE 0xcafe4all
@@ -77,6 +77,8 @@ struct bpf_test {
} result, result_unpriv;
enum bpf_prog_type prog_type;
uint8_t flags;
+ __u8 data[TEST_DATA_LEN];
+ void (*fill_helper)(struct bpf_test *self);
};
/* Note we want this to be 64 bit aligned so that the end of our array is
@@ -94,6 +96,62 @@ struct other_val {
long long bar;
};
+static void bpf_fill_ld_abs_vlan_push_pop(struct bpf_test *self)
+{
+ /* test: {skb->data[0], vlan_push} x 68 + {skb->data[0], vlan_pop} x 68 */
+#define PUSH_CNT 51
+ unsigned int len = BPF_MAXINSNS;
+ struct bpf_insn *insn = self->insns;
+ int i = 0, j, k = 0;
+
+ insn[i++] = BPF_MOV64_REG(BPF_REG_6, BPF_REG_1);
+loop:
+ for (j = 0; j < PUSH_CNT; j++) {
+ insn[i++] = BPF_LD_ABS(BPF_B, 0);
+ insn[i] = BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0x34, len - i - 2);
+ i++;
+ insn[i++] = BPF_MOV64_REG(BPF_REG_1, BPF_REG_6);
+ insn[i++] = BPF_MOV64_IMM(BPF_REG_2, 1);
+ insn[i++] = BPF_MOV64_IMM(BPF_REG_3, 2);
+ insn[i++] = BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
+ BPF_FUNC_skb_vlan_push),
+ insn[i] = BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, len - i - 2);
+ i++;
+ }
+
+ for (j = 0; j < PUSH_CNT; j++) {
+ insn[i++] = BPF_LD_ABS(BPF_B, 0);
+ insn[i] = BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0x34, len - i - 2);
+ i++;
+ insn[i++] = BPF_MOV64_REG(BPF_REG_1, BPF_REG_6);
+ insn[i++] = BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
+ BPF_FUNC_skb_vlan_pop),
+ insn[i] = BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, len - i - 2);
+ i++;
+ }
+ if (++k < 5)
+ goto loop;
+
+ for (; i < len - 1; i++)
+ insn[i] = BPF_ALU32_IMM(BPF_MOV, BPF_REG_0, 0xbef);
+ insn[len - 1] = BPF_EXIT_INSN();
+}
+
+static void bpf_fill_jump_around_ld_abs(struct bpf_test *self)
+{
+ struct bpf_insn *insn = self->insns;
+ unsigned int len = BPF_MAXINSNS;
+ int i = 0;
+
+ insn[i++] = BPF_MOV64_REG(BPF_REG_6, BPF_REG_1);
+ insn[i++] = BPF_LD_ABS(BPF_B, 0);
+ insn[i] = BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 10, len - i - 2);
+ i++;
+ while (i < len - 1)
+ insn[i++] = BPF_LD_ABS(BPF_B, 1);
+ insn[i] = BPF_EXIT_INSN();
+}
+
static struct bpf_test tests[] = {
{
"add+sub+mul",
@@ -11725,6 +11783,197 @@ static struct bpf_test tests[] = {
.result = ACCEPT,
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
},
+ {
+ "ld_abs: invalid op 1",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+ BPF_LD_ABS(BPF_DW, 0),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = REJECT,
+ .errstr = "unknown opcode",
+ },
+ {
+ "ld_abs: invalid op 2",
+ .insns = {
+ BPF_MOV32_IMM(BPF_REG_0, 256),
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+ BPF_LD_IND(BPF_DW, BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = REJECT,
+ .errstr = "unknown opcode",
+ },
+ {
+ "ld_abs: nmap reduced",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+ BPF_LD_ABS(BPF_H, 12),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0x806, 28),
+ BPF_LD_ABS(BPF_H, 12),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0x806, 26),
+ BPF_MOV32_IMM(BPF_REG_0, 18),
+ BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -64),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_10, -64),
+ BPF_LD_IND(BPF_W, BPF_REG_7, 14),
+ BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -60),
+ BPF_MOV32_IMM(BPF_REG_0, 280971478),
+ BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -56),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_10, -56),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_10, -60),
+ BPF_ALU32_REG(BPF_SUB, BPF_REG_0, BPF_REG_7),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 15),
+ BPF_LD_ABS(BPF_H, 12),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0x806, 13),
+ BPF_MOV32_IMM(BPF_REG_0, 22),
+ BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -56),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_10, -56),
+ BPF_LD_IND(BPF_H, BPF_REG_7, 14),
+ BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -52),
+ BPF_MOV32_IMM(BPF_REG_0, 17366),
+ BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -48),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_10, -48),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_10, -52),
+ BPF_ALU32_REG(BPF_SUB, BPF_REG_0, BPF_REG_7),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2),
+ BPF_MOV32_IMM(BPF_REG_0, 256),
+ BPF_EXIT_INSN(),
+ BPF_MOV32_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .data = {
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x08, 0x06, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ 0x10, 0xbf, 0x48, 0xd6, 0x43, 0xd6,
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = ACCEPT,
+ .retval = 256,
+ },
+ {
+ "ld_abs: div + abs, test 1",
+ .insns = {
+ BPF_ALU64_REG(BPF_MOV, BPF_REG_6, BPF_REG_1),
+ BPF_LD_ABS(BPF_B, 3),
+ BPF_ALU64_IMM(BPF_MOV, BPF_REG_2, 2),
+ BPF_ALU32_REG(BPF_DIV, BPF_REG_0, BPF_REG_2),
+ BPF_ALU64_REG(BPF_MOV, BPF_REG_8, BPF_REG_0),
+ BPF_LD_ABS(BPF_B, 4),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_8, BPF_REG_0),
+ BPF_LD_IND(BPF_B, BPF_REG_8, -70),
+ BPF_EXIT_INSN(),
+ },
+ .data = {
+ 10, 20, 30, 40, 50,
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = ACCEPT,
+ .retval = 10,
+ },
+ {
+ "ld_abs: div + abs, test 2",
+ .insns = {
+ BPF_ALU64_REG(BPF_MOV, BPF_REG_6, BPF_REG_1),
+ BPF_LD_ABS(BPF_B, 3),
+ BPF_ALU64_IMM(BPF_MOV, BPF_REG_2, 2),
+ BPF_ALU32_REG(BPF_DIV, BPF_REG_0, BPF_REG_2),
+ BPF_ALU64_REG(BPF_MOV, BPF_REG_8, BPF_REG_0),
+ BPF_LD_ABS(BPF_B, 128),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_8, BPF_REG_0),
+ BPF_LD_IND(BPF_B, BPF_REG_8, -70),
+ BPF_EXIT_INSN(),
+ },
+ .data = {
+ 10, 20, 30, 40, 50,
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = ACCEPT,
+ .retval = 0,
+ },
+ {
+ "ld_abs: div + abs, test 3",
+ .insns = {
+ BPF_ALU64_REG(BPF_MOV, BPF_REG_6, BPF_REG_1),
+ BPF_ALU64_IMM(BPF_MOV, BPF_REG_7, 0),
+ BPF_LD_ABS(BPF_B, 3),
+ BPF_ALU32_REG(BPF_DIV, BPF_REG_0, BPF_REG_7),
+ BPF_EXIT_INSN(),
+ },
+ .data = {
+ 10, 20, 30, 40, 50,
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = ACCEPT,
+ .retval = 0,
+ },
+ {
+ "ld_abs: div + abs, test 4",
+ .insns = {
+ BPF_ALU64_REG(BPF_MOV, BPF_REG_6, BPF_REG_1),
+ BPF_ALU64_IMM(BPF_MOV, BPF_REG_7, 0),
+ BPF_LD_ABS(BPF_B, 256),
+ BPF_ALU32_REG(BPF_DIV, BPF_REG_0, BPF_REG_7),
+ BPF_EXIT_INSN(),
+ },
+ .data = {
+ 10, 20, 30, 40, 50,
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = ACCEPT,
+ .retval = 0,
+ },
+ {
+ "ld_abs: vlan + abs, test 1",
+ .insns = { },
+ .data = {
+ 0x34,
+ },
+ .fill_helper = bpf_fill_ld_abs_vlan_push_pop,
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = ACCEPT,
+ .retval = 0xbef,
+ },
+ {
+ "ld_abs: vlan + abs, test 2",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+ BPF_LD_ABS(BPF_B, 0),
+ BPF_LD_ABS(BPF_H, 0),
+ BPF_LD_ABS(BPF_W, 0),
+ BPF_MOV64_REG(BPF_REG_7, BPF_REG_6),
+ BPF_MOV64_IMM(BPF_REG_6, 0),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
+ BPF_MOV64_IMM(BPF_REG_2, 1),
+ BPF_MOV64_IMM(BPF_REG_3, 2),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
+ BPF_FUNC_skb_vlan_push),
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_7),
+ BPF_LD_ABS(BPF_B, 0),
+ BPF_LD_ABS(BPF_H, 0),
+ BPF_LD_ABS(BPF_W, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 42),
+ BPF_EXIT_INSN(),
+ },
+ .data = {
+ 0x34,
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = ACCEPT,
+ .retval = 42,
+ },
+ {
+ "ld_abs: jump around ld_abs",
+ .insns = { },
+ .data = {
+ 10, 11,
+ },
+ .fill_helper = bpf_fill_jump_around_ld_abs,
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = ACCEPT,
+ .retval = 10,
+ },
};
static int probe_filter_length(const struct bpf_insn *fp)
@@ -11828,7 +12077,7 @@ static int create_map_in_map(void)
return outer_map_fd;
}
-static char bpf_vlog[32768];
+static char bpf_vlog[UINT_MAX >> 8];
static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
int *map_fds)
@@ -11839,6 +12088,9 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
int *fixup_prog = test->fixup_prog;
int *fixup_map_in_map = test->fixup_map_in_map;
+ if (test->fill_helper)
+ test->fill_helper(test);
+
/* Allocating HTs with 1 elem is fine here, since we only test
* for verifier and not do a runtime lookup, so the only thing
* that really matters is value size in this case.
@@ -11888,10 +12140,8 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
int *passes, int *errors)
{
int fd_prog, expected_ret, reject_from_alignment;
+ int prog_len, prog_type = test->prog_type;
struct bpf_insn *prog = test->insns;
- int prog_len = probe_filter_length(prog);
- char data_in[TEST_DATA_LEN] = {};
- int prog_type = test->prog_type;
int map_fds[MAX_NR_MAPS];
const char *expected_err;
uint32_t retval;
@@ -11901,6 +12151,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
map_fds[i] = -1;
do_test_fixup(test, prog, map_fds);
+ prog_len = probe_filter_length(prog);
fd_prog = bpf_verify_program(prog_type ? : BPF_PROG_TYPE_SOCKET_FILTER,
prog, prog_len, test->flags & F_LOAD_WITH_STRICT_ALIGNMENT,
@@ -11940,8 +12191,9 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
}
if (fd_prog >= 0) {
- err = bpf_prog_test_run(fd_prog, 1, data_in, sizeof(data_in),
- NULL, NULL, &retval, NULL);
+ err = bpf_prog_test_run(fd_prog, 1, test->data,
+ sizeof(test->data), NULL, NULL,
+ &retval, NULL);
if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
printf("Unexpected bpf_prog_test_run error\n");
goto fail_log;
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 01/12] bpf: prefix cbpf internal helpers with bpf_
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>
No change in functionality, just remove the '__' prefix and replace it
with a 'bpf_' prefix instead. We later on add a couple of more helpers
for cBPF and keeping the scheme with '__' is suboptimal there.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
net/core/filter.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index d3781da..07fe378 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -112,12 +112,12 @@ int sk_filter_trim_cap(struct sock *sk, struct sk_buff *skb, unsigned int cap)
}
EXPORT_SYMBOL(sk_filter_trim_cap);
-BPF_CALL_1(__skb_get_pay_offset, struct sk_buff *, skb)
+BPF_CALL_1(bpf_skb_get_pay_offset, struct sk_buff *, skb)
{
return skb_get_poff(skb);
}
-BPF_CALL_3(__skb_get_nlattr, struct sk_buff *, skb, u32, a, u32, x)
+BPF_CALL_3(bpf_skb_get_nlattr, struct sk_buff *, skb, u32, a, u32, x)
{
struct nlattr *nla;
@@ -137,7 +137,7 @@ BPF_CALL_3(__skb_get_nlattr, struct sk_buff *, skb, u32, a, u32, x)
return 0;
}
-BPF_CALL_3(__skb_get_nlattr_nest, struct sk_buff *, skb, u32, a, u32, x)
+BPF_CALL_3(bpf_skb_get_nlattr_nest, struct sk_buff *, skb, u32, a, u32, x)
{
struct nlattr *nla;
@@ -161,13 +161,13 @@ BPF_CALL_3(__skb_get_nlattr_nest, struct sk_buff *, skb, u32, a, u32, x)
return 0;
}
-BPF_CALL_0(__get_raw_cpu_id)
+BPF_CALL_0(bpf_get_raw_cpu_id)
{
return raw_smp_processor_id();
}
static const struct bpf_func_proto bpf_get_raw_smp_processor_id_proto = {
- .func = __get_raw_cpu_id,
+ .func = bpf_get_raw_cpu_id,
.gpl_only = false,
.ret_type = RET_INTEGER,
};
@@ -317,16 +317,16 @@ static bool convert_bpf_extensions(struct sock_filter *fp,
/* Emit call(arg1=CTX, arg2=A, arg3=X) */
switch (fp->k) {
case SKF_AD_OFF + SKF_AD_PAY_OFFSET:
- *insn = BPF_EMIT_CALL(__skb_get_pay_offset);
+ *insn = BPF_EMIT_CALL(bpf_skb_get_pay_offset);
break;
case SKF_AD_OFF + SKF_AD_NLATTR:
- *insn = BPF_EMIT_CALL(__skb_get_nlattr);
+ *insn = BPF_EMIT_CALL(bpf_skb_get_nlattr);
break;
case SKF_AD_OFF + SKF_AD_NLATTR_NEST:
- *insn = BPF_EMIT_CALL(__skb_get_nlattr_nest);
+ *insn = BPF_EMIT_CALL(bpf_skb_get_nlattr_nest);
break;
case SKF_AD_OFF + SKF_AD_CPU:
- *insn = BPF_EMIT_CALL(__get_raw_cpu_id);
+ *insn = BPF_EMIT_CALL(bpf_get_raw_cpu_id);
break;
case SKF_AD_OFF + SKF_AD_RANDOM:
*insn = BPF_EMIT_CALL(bpf_user_rnd_u32);
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next 00/12] Move ld_abs/ld_ind to native BPF
From: Daniel Borkmann @ 2018-05-03 1:05 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
This set simplifies BPF JITs significantly by moving ld_abs/ld_ind
to native BPF, for details see individual patches. Main rationale
is in patch 'implement ld_abs/ld_ind in native bpf'. Thanks!
Daniel Borkmann (12):
bpf: prefix cbpf internal helpers with bpf_
bpf: migrate ebpf ld_abs/ld_ind tests to test_verifier
bpf: implement ld_abs/ld_ind in native bpf
bpf: add skb_load_bytes_relative helper
bpf, x64: remove ld_abs/ld_ind
bpf, arm64: remove ld_abs/ld_ind
bpf, sparc64: remove ld_abs/ld_ind
bpf, arm32: remove ld_abs/ld_ind
bpf, mips64: remove ld_abs/ld_ind
bpf, ppc64: remove ld_abs/ld_ind
bpf, s390x: remove ld_abs/ld_ind
bpf: sync tools bpf.h uapi header
arch/arm/net/bpf_jit_32.c | 77 --------
arch/arm64/net/bpf_jit_comp.c | 65 ------
arch/mips/net/ebpf_jit.c | 104 ----------
arch/powerpc/net/Makefile | 2 +-
arch/powerpc/net/bpf_jit64.h | 37 +---
arch/powerpc/net/bpf_jit_asm64.S | 180 -----------------
arch/powerpc/net/bpf_jit_comp64.c | 109 +---------
arch/s390/net/Makefile | 2 +-
arch/s390/net/bpf_jit.S | 116 -----------
arch/s390/net/bpf_jit.h | 20 +-
arch/s390/net/bpf_jit_comp.c | 127 ++----------
arch/sparc/net/Makefile | 5 +-
arch/sparc/net/bpf_jit_64.h | 29 ---
arch/sparc/net/bpf_jit_asm_64.S | 162 ---------------
arch/sparc/net/bpf_jit_comp_64.c | 79 +-------
arch/x86/net/Makefile | 4 +-
arch/x86/net/bpf_jit.S | 154 ---------------
arch/x86/net/bpf_jit_comp.c | 144 +-------------
include/linux/bpf.h | 4 +-
include/linux/filter.h | 4 +-
include/uapi/linux/bpf.h | 33 +++-
kernel/bpf/core.c | 96 +--------
kernel/bpf/verifier.c | 24 +++
lib/test_bpf.c | 212 --------------------
net/core/filter.c | 296 +++++++++++++++++++++++++---
tools/include/uapi/linux/bpf.h | 33 +++-
tools/testing/selftests/bpf/test_verifier.c | 266 ++++++++++++++++++++++++-
27 files changed, 669 insertions(+), 1715 deletions(-)
delete mode 100644 arch/powerpc/net/bpf_jit_asm64.S
delete mode 100644 arch/s390/net/bpf_jit.S
delete mode 100644 arch/sparc/net/bpf_jit_asm_64.S
delete mode 100644 arch/x86/net/bpf_jit.S
--
2.9.5
^ permalink raw reply
* pull-request: bpf 2018-05-03
From: Daniel Borkmann @ 2018-05-03 0:37 UTC (permalink / raw)
To: davem; +Cc: daniel, ast, netdev
Hi David,
The following pull-request contains BPF updates for your *net* tree.
The main changes are:
1) Several BPF sockmap fixes mostly related to bugs in error path
handling, that is, a bug in updating the scatterlist length /
offset accounting, a missing sk_mem_uncharge() in redirect
error handling, and a bug where the outstanding bytes counter
sg_size was not zeroed, from John.
2) Fix two memory leaks in the x86-64 BPF JIT, one in an error
path where we still don't converge after image was allocated
and another one where BPF calls are used and JIT passes don't
converge, from Daniel.
3) Minor fix in BPF selftests where in test_stacktrace_build_id()
we drop useless args in urandom_read and we need to add a missing
newline in a CHECK() error message, from Song.
Please consider pulling these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
Thanks a lot!
----------------------------------------------------------------
The following changes since commit 25eb0ea7174c6e84f21fa59dccbddd0318b17b12:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf (2018-04-25 22:55:33 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
for you to fetch changes up to b5b6ff730253ab68ec230e239c4245cb1e8a5397:
Merge branch 'bpf-sockmap-fixes' (2018-05-02 15:30:46 -0700)
----------------------------------------------------------------
Alexei Starovoitov (2):
Merge branch 'x86-bpf-jit-fixes'
Merge branch 'bpf-sockmap-fixes'
Daniel Borkmann (2):
bpf, x64: fix memleak when not converging after image
bpf, x64: fix memleak when not converging on calls
John Fastabend (4):
bpf: fix uninitialized variable in bpf tools
bpf: sockmap, fix scatterlist update on error path in send with apply
bpf: sockmap, zero sg_size on error when buffer is released
bpf: sockmap, fix error handling in redirect failures
Song Liu (1):
bpf: minor fix to selftest test_stacktrace_build_id()
arch/x86/net/bpf_jit_comp.c | 6 ++--
kernel/bpf/sockmap.c | 48 +++++++++++++++++---------------
tools/bpf/bpf_dbg.c | 7 +++--
tools/testing/selftests/bpf/test_progs.c | 4 +--
4 files changed, 36 insertions(+), 29 deletions(-)
^ permalink raw reply
* Re: [v2 PATCH 1/1] tg3: fix meaningless hw_stats reading after tg3_halt memset 0 hw_stats
From: Zumeng Chen @ 2018-05-03 0:30 UTC (permalink / raw)
To: Michael Chan
Cc: Netdev, open list, Siva Reddy Kallam,
prashant.sreedharan@broadcom.com, David Miller, Zumeng Chen
In-Reply-To: <CACKFLinOzi+F9j+gbBHjkGdMdXTqpfMRLUwqx=zUpVg76zGxNA@mail.gmail.com>
On 2018年05月03日 01:32, Michael Chan wrote:
> On Wed, May 2, 2018 at 3:27 AM, Zumeng Chen <zumeng.chen@gmail.com> wrote:
>> On 2018年05月02日 13:12, Michael Chan wrote:
>>> On Tue, May 1, 2018 at 5:42 PM, Zumeng Chen <zumeng.chen@gmail.com> wrote:
>>>
>>>> diff --git a/drivers/net/ethernet/broadcom/tg3.h
>>>> b/drivers/net/ethernet/broadcom/tg3.h
>>>> index 3b5e98e..c61d83c 100644
>>>> --- a/drivers/net/ethernet/broadcom/tg3.h
>>>> +++ b/drivers/net/ethernet/broadcom/tg3.h
>>>> @@ -3102,6 +3102,7 @@ enum TG3_FLAGS {
>>>> TG3_FLAG_ROBOSWITCH,
>>>> TG3_FLAG_ONE_DMA_AT_ONCE,
>>>> TG3_FLAG_RGMII_MODE,
>>>> + TG3_FLAG_HALT,
>>> I think you should be able to use the existing INIT_COMPLETE flag
>>
>> No, it will bring the uncertain factors into the existed complicate logic
>> of INIT_COMPLETE.
>> And I think it's very simple logic here to fix the meaningless hw_stats
>> reading and the problem
>> of commit f5992b72. I even suspect if you have read INIT_COMPLETE related
>> codes carefully.
>>
> We should use an existing flag whenever appropriate
I disagree. This is sort of blahblah...
> , instead of adding
> yet another flag to do similar things. I've looked at the code briefly
> and believe that INIT_COMPLETE will work.
When we fix a problem, we'd better think if we introduce a new one.
> If you think it won't work,
> please be specific and point out why it won't work. Thanks.
I don't care if it work or not, I directly feel it's a bad idea.
INIT_COMPLETE include a lot of network stuffs, it's not simple hardware
reset related.
Here again,
My fix logic is very simple to fix the problem I met, I think this is
how Linux works together
with such a lot of thing, which means clear, simple, and robust for
every unit, we re-unite
them eco-systematically.
Finally, it's yours, so be it.
Cheers,
Zumeng
^ permalink raw reply
* Re: Silently dropped UDP packets on kernel 4.14
From: Kristian Evensen @ 2018-05-02 23:54 UTC (permalink / raw)
To: Florian Westphal; +Cc: Netfilter Development Mailing list, Network Development
In-Reply-To: <CAKfDRXhkXvF==6LQFOG3Rf4dH4QVVUxR=1cXZoM2FPQksGdxwg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 12216 bytes --]
Hello,
On Wed, May 2, 2018 at 12:42 AM, Kristian Evensen
<kristian.evensen@gmail.com> wrote:
> My knowledge of the conntrack/nat subsystem is not that great, and I
> don't know the implications of what I am about to suggest. However,
> considering that the two packets represent the same flow, wouldn't it
> be possible to apply the existing nat-mapping to the second packet,
> and then let the second packet pass?
I have spent the day today trying to solve my problem and I think I am
almost there. I have attached my work in progress patch to this email
if anyone wants to take a look (for kernel 4.14).
I went for the early-insert approached and have patched
nfnetlink_queue to perform an insert if no conntrack entry is found
when the verdict is passed from user-space. If a conntrack entry is
found, then I replace the ct attached to the skb with the existing
conntrack entry. I have verified that my approach works by
artificially delaying the verdict from my application for the second
packet (so that the first packet has passed all netfilter hooks).
After replacing the ct, the second packet is handled correctly and
sent over the wire.
However, something goes wrong after the conntrack entry has been
inserted (using nf_conntrack_hash_check_insert()). At some random (but
very short) time after I see a couple of "early insert ..."/"early
insert confirmed", I get an RCU-stall. The trace for the stall looks
as follows:
[ 105.420024] INFO: rcu_sched self-detected stall on CPU
[ 105.425191] 2-...: (5999 ticks this GP) idle=12a/140000000000001/0
softirq=2674/2674 fqs=2543
[ 105.433845] (t=6001 jiffies g=587 c=586 q=5896)
[ 105.438545] CPU: 2 PID: 3632 Comm: dlb Not tainted 4.14.36 #0
[ 105.444261] Stack : 00000000 00000000 00000000 00000000 805c7ada
00000031 00000000 8052c588
[ 105.452610] 8fd48ff4 805668e7 804f5a2c 00000002 00000e30
00000001 8fc11d20 00000007
[ 105.460957] 00000000 00000000 805c0000 00007448 00000000
0000016e 00000007 00000000
[ 105.469303] 00000000 80570000 0006b111 00000000 80000000
00000000 80590000 804608e4
[ 105.477650] 8056c2c0 8056408c 000000e0 80560000 00000000
8027c418 00000008 805c0008
[ 105.485996] ...
[ 105.488437] Call Trace:
[ 105.490909] [<800103f8>] show_stack+0x58/0x100
[ 105.495363] [<8043c1dc>] dump_stack+0x9c/0xe0
[ 105.499707] [<8000d938>] arch_trigger_cpumask_backtrace+0x50/0x78
[ 105.505785] [<80083540>] rcu_dump_cpu_stacks+0xc4/0x134
[ 105.510991] [<800829c4>] rcu_check_callbacks+0x310/0x814
[ 105.516295] [<80085f04>] update_process_times+0x34/0x70
[ 105.521522] [<8009691c>] tick_handle_periodic+0x34/0xd0
[ 105.526749] [<802f6e98>] gic_compare_interrupt+0x48/0x58
[ 105.532047] [<80076450>] handle_percpu_devid_irq+0xbc/0x1a8
[ 105.537619] [<800707c0>] generic_handle_irq+0x40/0x58
[ 105.542679] [<8023a274>] gic_handle_local_int+0x84/0xd0
[ 105.547886] [<8023a434>] gic_irq_dispatch+0x10/0x20
[ 105.552747] [<800707c0>] generic_handle_irq+0x40/0x58
[ 105.557806] [<804591b4>] do_IRQ+0x1c/0x2c
[ 105.561805] [<802394bc>] plat_irq_dispatch+0xfc/0x138
[ 105.566839] [<8000b508>] except_vec_vi_end+0xb8/0xc4
[ 105.571904] [<8f200ca4>] nf_conntrack_lock+0x28c/0x440 [nf_conntrack]
[ 105.578341] ------------[ cut here ]------------
[ 105.582948] WARNING: CPU: 2 PID: 3632 at kernel/smp.c:416
smp_call_function_many+0xc8/0x3bc
[ 105.591257] Modules linked in: rt2800pci rt2800mmio rt2800lib
qcserial ppp_async option usb_wwan rt2x00pci rt2x00mmio rt2x00lib
rndis_host qmi_wwan ppp_generic nf_nat_pptp nf_conntrack_pptp
nf_conntrack_ipv6p
[ 105.662308] nf_nat_snmp_basic nf_nat_sip nf_nat_redirect
nf_nat_proto_gre nf_nat_masquerade_ipv4 nf_nat_irc nf_conntrack_ipv4
nf_nat_ipv4 nf_nat_h323 nf_nat_ftp nf_nat_amanda nf_nat nf_log_ipv4
nf_flow_tablt
[ 105.733288] ip_set_hash_netiface ip_set_hash_netport
ip_set_hash_netnet ip_set_hash_net ip_set_hash_netportnet
ip_set_hash_mac ip_set_hash_ipportnet ip_set_hash_ipportip
ip_set_hash_ipport ip_set_hash_ipmarm
[ 105.804724] ohci_hcd ehci_platform sd_mod scsi_mod ehci_hcd
gpio_button_hotplug usbcore nls_base usb_common mii
[ 105.814899] CPU: 2 PID: 3632 Comm: dlb Not tainted 4.14.36 #0
[ 105.820615] Stack : 00000000 00000000 00000000 00000000 805c7ada
00000031 00000000 8052c588
[ 105.828961] 8fd48ff4 805668e7 804f5a2c 00000002 00000e30
00000001 8fc11c88 00000007
[ 105.837308] 00000000 00000000 805c0000 00008590 00000000
0000018d 00000007 00000000
[ 105.845654] 00000000 80570000 000c6f33 00000000 80000000
00000000 80590000 8009e304
[ 105.854001] 00000009 000001a0 8000d0fc 00000000 00000000
8027c418 00000008 805c0008
[ 105.862348] ...
[ 105.864786] Call Trace:
[ 105.867230] [<800103f8>] show_stack+0x58/0x100
[ 105.871662] [<8043c1dc>] dump_stack+0x9c/0xe0
[ 105.876009] [<8002e190>] __warn+0xe0/0x114
[ 105.880091] [<8002e254>] warn_slowpath_null+0x1c/0x28
[ 105.885124] [<8009e304>] smp_call_function_many+0xc8/0x3bc
[ 105.890591] [<8000d950>] arch_trigger_cpumask_backtrace+0x68/0x78
[ 105.896663] [<80083540>] rcu_dump_cpu_stacks+0xc4/0x134
[ 105.901869] [<800829c4>] rcu_check_callbacks+0x310/0x814
[ 105.907164] [<80085f04>] update_process_times+0x34/0x70
[ 105.912374] [<8009691c>] tick_handle_periodic+0x34/0xd0
[ 105.917585] [<802f6e98>] gic_compare_interrupt+0x48/0x58
[ 105.922878] [<80076450>] handle_percpu_devid_irq+0xbc/0x1a8
[ 105.928434] [<800707c0>] generic_handle_irq+0x40/0x58
[ 105.933474] [<8023a274>] gic_handle_local_int+0x84/0xd0
[ 105.938681] [<8023a434>] gic_irq_dispatch+0x10/0x20
[ 105.943542] [<800707c0>] generic_handle_irq+0x40/0x58
[ 105.948581] [<804591b4>] do_IRQ+0x1c/0x2c
[ 105.952580] [<802394bc>] plat_irq_dispatch+0xfc/0x138
[ 105.957613] [<8000b508>] except_vec_vi_end+0xb8/0xc4
[ 105.962602] [<8f200ca4>] nf_conntrack_lock+0x28c/0x440 [nf_conntrack]
[ 105.969038] ---[ end trace 0b9e3b8a6909a0fb ]---
[ 105.973638] INFO: rcu_sched detected stalls on CPUs/tasks:
[ 105.979149] 2-...: (6054 ticks this GP) idle=12a/140000000000000/0
softirq=2674/2674 fqs=2544
[ 105.987818] (detected by 3, t=6056 jiffies, g=587, c=586, q=5896)
[ 105.993996] ------------[ cut here ]------------
[ 105.998615] WARNING: CPU: 3 PID: 0 at kernel/smp.c:291
smp_call_function_single+0xc0/0x18c
[ 106.006841] Modules linked in: rt2800pci rt2800mmio rt2800lib
qcserial ppp_async option usb_wwan rt2x00pci rt2x00mmio rt2x00lib
rndis_host qmi_wwan ppp_generic nf_nat_pptp nf_conntrack_pptp
nf_conntrack_ipv6p
[ 106.077960] nf_nat_snmp_basic nf_nat_sip nf_nat_redirect
nf_nat_proto_gre nf_nat_masquerade_ipv4 nf_nat_irc nf_conntrack_ipv4
nf_nat_ipv4 nf_nat_h323 nf_nat_ftp nf_nat_amanda nf_nat nf_log_ipv4
nf_flow_tablt
[ 106.148997] ip_set_hash_netiface ip_set_hash_netport
ip_set_hash_netnet ip_set_hash_net ip_set_hash_netportnet
ip_set_hash_mac ip_set_hash_ipportnet ip_set_hash_ipportip
ip_set_hash_ipport ip_set_hash_ipmarm
[ 106.220489] ohci_hcd ehci_platform sd_mod scsi_mod ehci_hcd
gpio_button_hotplug usbcore nls_base usb_common mii
[ 106.230686] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G W
4.14.36 #0
[ 106.237876] Stack : 00000000 00000000 00000000 00000000 805c7ada
00000042 00000000 8052c588
[ 106.246232] 8fc44e74 805668e7 804f5a2c 00000003 00000000
00000001 8fc15c80 00000007
[ 106.254587] 00000000 00000000 805c0000 000098c0 00000000
000001b3 00000007 00000000
[ 106.262942] 00000000 80570000 0003851e 00000000 00000000
00000000 80590000 8009dfb0
[ 106.271299] 00000009 00000123 000000e0 80560000 00000001
8027c418 0000000c 805c000c
[ 106.279653] ...
[ 106.282097] Call Trace:
[ 106.284563] [<800103f8>] show_stack+0x58/0x100
[ 106.289026] [<8043c1dc>] dump_stack+0x9c/0xe0
[ 106.293380] [<8002e190>] __warn+0xe0/0x114
[ 106.297467] [<8002e254>] warn_slowpath_null+0x1c/0x28
[ 106.302506] [<8009dfb0>] smp_call_function_single+0xc0/0x18c
[ 106.308150] [<8000d950>] arch_trigger_cpumask_backtrace+0x68/0x78
[ 106.314235] [<80083540>] rcu_dump_cpu_stacks+0xc4/0x134
[ 106.319445] [<80082c8c>] rcu_check_callbacks+0x5d8/0x814
[ 106.324757] [<80085f04>] update_process_times+0x34/0x70
[ 106.329992] [<8009691c>] tick_handle_periodic+0x34/0xd0
[ 106.335230] [<802f6e98>] gic_compare_interrupt+0x48/0x58
[ 106.340537] [<80076450>] handle_percpu_devid_irq+0xbc/0x1a8
[ 106.346118] [<800707c0>] generic_handle_irq+0x40/0x58
[ 106.351190] [<8023a274>] gic_handle_local_int+0x84/0xd0
[ 106.356402] [<8023a434>] gic_irq_dispatch+0x10/0x20
[ 106.361270] [<800707c0>] generic_handle_irq+0x40/0x58
[ 106.366339] [<804591b4>] do_IRQ+0x1c/0x2c
[ 106.370344] [<802394bc>] plat_irq_dispatch+0xfc/0x138
[ 106.375383] [<8000b508>] except_vec_vi_end+0xb8/0xc4
[ 106.380336] [<8000ced0>] r4k_wait_irqoff+0x1c/0x24
[ 106.385146] [<80065f6c>] do_idle+0xe4/0x168
[ 106.389322] [<800661e8>] cpu_startup_entry+0x24/0x2c
[ 106.394275] ---[ end trace 0b9e3b8a6909a0fc ]---
[ 106.398888] CPU: 2 PID: 3632 Comm: dlb Tainted: G W 4.14.36 #0
[ 106.405819] task: 8fd48c80 task.stack: 8e274000
[ 106.410324] $ 0 : 00000000 00000001 814b7588 00000001
[ 106.415544] $ 4 : 09036633 80564198 71804fce d0db5a72
[ 106.420763] $ 8 : 00000000 8aee9400 814bee18 00157558
[ 106.425982] $12 : 00000000 00000000 fffffffe 00000114
[ 106.431201] $16 : 00005b78 8e275998 8e874540 80560000
[ 106.436421] $20 : 00000001 8f210000 00005a31 8f210000
[ 106.441639] $24 : 00000000 80015e5c
[ 106.446857] $28 : 8e274000 8e275940 8f210000 8f201634
[ 106.452078] Hi : 00005a31
[ 106.454942] Lo : 6e620000
[ 106.457912] epc : 8f2017f4 nf_conntrack_tuple_taken+0x258/0x308
[nf_conntrack]
[ 106.465299] ra : 8f201634 nf_conntrack_tuple_taken+0x98/0x308
[nf_conntrack]
[ 106.472571] Status: 11007c03 KERNEL EXL IE
[ 106.476748] Cause : 50800400 (ExcCode 00)
[ 106.480738] PrId : 0001992f (MIPS 1004Kc)
[ 106.484819] CPU: 2 PID: 3632 Comm: dlb Tainted: G W 4.14.36 #0
[ 106.491742] Stack : 00000000 00000000 00000000 00000000 805c7ada
0000003f 00000000 8052c588
[ 106.500088] 8fd48ff4 805668e7 804f5a2c 00000002 00000e30
00000001 8fc11d68 00000007
[ 106.508435] 00000000 00000000 805c0000 0000a498 00000000
000001e3 00000007 00000000
[ 106.516779] 00000000 80570000 000765d3 00000000 80000000
00000000 80590000 8000d0fc
[ 106.525126] 804fce50 805574e0 804faa5c 80570000 00000002
8027c418 00000008 805c0008
[ 106.533473] ...
[ 106.535911] Call Trace:
[ 106.538356] [<800103f8>] show_stack+0x58/0x100
[ 106.542788] [<8043c1dc>] dump_stack+0x9c/0xe0
[ 106.547130] [<8009dccc>] flush_smp_call_function_queue+0x144/0x1d8
[ 106.553294] [<80015830>] ipi_call_interrupt+0x10/0x20
[ 106.558336] [<80071380>] __handle_irq_event_percpu+0x78/0x18c
[ 106.564061] [<800714b4>] handle_irq_event_percpu+0x20/0x64
[ 106.569528] [<80071558>] handle_irq_event+0x60/0xa8
[ 106.574388] [<800757d0>] handle_edge_irq+0x204/0x25c
[ 106.579337] [<800707c0>] generic_handle_irq+0x40/0x58
[ 106.584377] [<8023a3c8>] gic_handle_shared_int+0x108/0x164
[ 106.589844] [<800707c0>] generic_handle_irq+0x40/0x58
[ 106.594883] [<804591b4>] do_IRQ+0x1c/0x2c
[ 106.598880] [<802394bc>] plat_irq_dispatch+0xfc/0x138
[ 106.603913] [<8000b508>] except_vec_vi_end+0xb8/0xc4
[ 106.608901] [<8f2017f4>] nf_conntrack_tuple_taken+0x258/0x308 [nf_conntrack]
[ 106.615963] [<8f1fcd88>] nf_ct_nat_ext_add+0x7ac/0x908 [nf_nat]
I instrumented nf_conntrack_tuple_taken() and it seems that what
happens, is that tuple_taken() spins on the "goto begin"-part. I am
probably doing something wrong with my insert, but I am not able to
see what. Is anyone able to see what is wrong, or can provide me with
pointers on where to look? When looking at the syslog, I also noticed
the following:
[ 45.394817] early insert: tuple 17 192.168.6.2:36376 -> 1.1.1.1:53
[ 45.401056] early insert confirmed
[ 45.405468] early insert: tuple 17 192.168.6.2:36376 -> 1.1.1.1:53
[ 45.411996] early insert confirmed
I.e., the entry created by the first packet is not found when the
second packet is handled. Could it be some synchronization step that I
am missing?
Thanks in advance for any help,
Kristian
[-- Attachment #2: 0001-NFQUEUE-early-insert.patch --]
[-- Type: text/x-patch, Size: 3148 bytes --]
From cfb4e8187940b89acf163a7f4181bf04abd333d0 Mon Sep 17 00:00:00 2001
From: Kristian Evensen <kristian.evensen@gmail.com>
Date: Tue, 1 May 2018 22:44:16 +0200
Subject: [PATCH] NFQUEUE early insert
---
net/netfilter/nfnetlink_queue.c | 69 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index c97966298..254866d5c 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -43,6 +43,9 @@
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
#include <net/netfilter/nf_conntrack.h>
+#include <net/netfilter/nf_conntrack_core.h>
+#include <net/netfilter/nf_conntrack_l3proto.h>
+#include <net/netfilter/nf_conntrack_l4proto.h>
#endif
#define NFQNL_QMAX_DEFAULT 1024
@@ -1151,6 +1154,64 @@ static int nfqa_parse_bridge(struct nf_queue_entry *entry,
return 0;
}
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+static void nfqnl_update_ct(struct net *net, struct sk_buff *skb)
+{
+ struct nf_conn *ct = NULL;
+ enum ip_conntrack_info ctinfo;
+ const struct nf_conntrack_l3proto *l3proto;
+ const struct nf_conntrack_l4proto *l4proto;
+ u_int16_t l3num;
+ unsigned int dataoff;
+ u_int8_t l4num;
+ struct nf_conntrack_tuple tuple;
+ struct nf_conntrack_tuple_hash *h;
+
+ ct = nf_ct_get(skb, &ctinfo);
+ l3num = nf_ct_l3num(ct);
+ l3proto = nf_ct_l3proto_find_get(l3num);
+
+ if (l3proto->get_l4proto(skb, skb_network_offset(skb), &dataoff,
+ &l4num) <= 0) {
+ pr_info("Failed to get l4 protonum\n");
+ return;
+ }
+
+ l4proto = nf_ct_l4proto_find_get(l3num, l4num);
+
+ if (!nf_ct_get_tuple(skb, skb_network_offset(skb), dataoff, l3num,
+ l4num, net, &tuple, l3proto, l4proto)) {
+ pr_info("Failed to get tuple\n");
+ return;
+ }
+
+#if IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES)
+ h = nf_conntrack_find_get(net, &(ct->zone), &tuple);
+#else
+ h = nf_conntrack_find_get(net, NULL, &tuple);
+#endif
+
+ if (!h) {
+ pr_info("early insert: tuple %u %pI4:%hu -> %pI4:%hu\n",
+ tuple.dst.protonum, &(tuple.src.u3.ip), ntohs(tuple.src.u.all),
+ &(tuple.dst.u3.ip), ntohs(tuple.dst.u.all));
+
+ rcu_read_lock();
+ if (!nf_conntrack_hash_check_insert(ct)) {
+ pr_info("early insert confirmed\n");
+ }
+ rcu_read_unlock();
+ } else {
+ pr_info("replace: tuple %u %pI4:%hu -> %pI4:%hu\n",
+ tuple.dst.protonum, &(tuple.src.u3.ip), ntohs(tuple.src.u.all),
+ &(tuple.dst.u3.ip), ntohs(tuple.dst.u.all));
+ nf_ct_put(ct);
+ ct = nf_ct_tuplehash_to_ctrack(h);
+ nf_ct_set(skb, ct, IP_CT_NEW);
+ }
+}
+#endif
+
static int nfqnl_recv_verdict(struct net *net, struct sock *ctnl,
struct sk_buff *skb,
const struct nlmsghdr *nlh,
@@ -1213,6 +1274,14 @@ static int nfqnl_recv_verdict(struct net *net, struct sock *ctnl,
if (nfqa[NFQA_MARK])
entry->skb->mark = ntohl(nla_get_be32(nfqa[NFQA_MARK]));
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+ nf_ct_get(entry->skb, &ctinfo);
+
+ if (ctinfo == IP_CT_NEW && verdict != NF_STOLEN && verdict != NF_DROP) {
+ nfqnl_update_ct(net, entry->skb);
+ }
+#endif
+
nf_reinject(entry, verdict);
return 0;
}
--
2.14.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox