| Message ID | 20190116011922.4420-1-willemdebruijn.kernel@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [net] bpf: in __bpf_redirect_no_mac pull mac only if present | expand |
On Tue, Jan 15, 2019 at 08:19:22PM -0500, Willem de Bruijn wrote: > From: Willem de Bruijn <willemb@google.com> > > Syzkaller was able to construct a packet of negative length by > redirecting from bpf_prog_test_run_skb with BPF_PROG_TYPE_LWT_XMIT: > > BUG: KASAN: slab-out-of-bounds in memcpy include/linux/string.h:345 [inline] > BUG: KASAN: slab-out-of-bounds in skb_copy_from_linear_data include/linux/skbuff.h:3421 [inline] > BUG: KASAN: slab-out-of-bounds in __pskb_copy_fclone+0x2dd/0xeb0 net/core/skbuff.c:1395 > Read of size 4294967282 at addr ffff8801d798009c by task syz-executor2/12942 > > kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412 > check_memory_region_inline mm/kasan/kasan.c:260 [inline] > check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267 > memcpy+0x23/0x50 mm/kasan/kasan.c:302 > memcpy include/linux/string.h:345 [inline] > skb_copy_from_linear_data include/linux/skbuff.h:3421 [inline] > __pskb_copy_fclone+0x2dd/0xeb0 net/core/skbuff.c:1395 > __pskb_copy include/linux/skbuff.h:1053 [inline] > pskb_copy include/linux/skbuff.h:2904 [inline] > skb_realloc_headroom+0xe7/0x120 net/core/skbuff.c:1539 > ipip6_tunnel_xmit net/ipv6/sit.c:965 [inline] > sit_tunnel_xmit+0xe1b/0x30d0 net/ipv6/sit.c:1029 > __netdev_start_xmit include/linux/netdevice.h:4325 [inline] > netdev_start_xmit include/linux/netdevice.h:4334 [inline] > xmit_one net/core/dev.c:3219 [inline] > dev_hard_start_xmit+0x295/0xc90 net/core/dev.c:3235 > __dev_queue_xmit+0x2f0d/0x3950 net/core/dev.c:3805 > dev_queue_xmit+0x17/0x20 net/core/dev.c:3838 > __bpf_tx_skb net/core/filter.c:2016 [inline] > __bpf_redirect_common net/core/filter.c:2054 [inline] > __bpf_redirect+0x5cf/0xb20 net/core/filter.c:2061 > ____bpf_clone_redirect net/core/filter.c:2094 [inline] > bpf_clone_redirect+0x2f6/0x490 net/core/filter.c:2066 > bpf_prog_41f2bcae09cd4ac3+0xb25/0x1000 > > The generated test constructs a packet with mac header, network > header, skb->data pointing to network header and skb->len 0. > > Redirecting to a sit0 through __bpf_redirect_no_mac pulls the > mac length, even though skb->data already is at skb->network_header. > bpf_prog_test_run_skb has already pulled it as LWT_XMIT !is_l2. > > Update the offset calculation to pull only if skb->data differs > from skb->network_header, which is not true in this case. > > The test itself can be run only from commit 1cf1cae963c2 ("bpf: > introduce BPF_PROG_TEST_RUN command"), but the same type of packets > with skb at network header could already be built from lwt xmit hooks, > so this fix is more relevant to that commit. > > Also set the mac header on redirect from LWT_XMIT, as even after this > change to __bpf_redirect_no_mac that field is expected to be set, but > is not yet in ip_finish_output2. LGTM. Acked-by: Martin KaFai Lau <kafai@fb.com>
On 01/16/2019 02:19 AM, Willem de Bruijn wrote: > From: Willem de Bruijn <willemb@google.com> > > Syzkaller was able to construct a packet of negative length by > redirecting from bpf_prog_test_run_skb with BPF_PROG_TYPE_LWT_XMIT: > > BUG: KASAN: slab-out-of-bounds in memcpy include/linux/string.h:345 [inline] > BUG: KASAN: slab-out-of-bounds in skb_copy_from_linear_data include/linux/skbuff.h:3421 [inline] > BUG: KASAN: slab-out-of-bounds in __pskb_copy_fclone+0x2dd/0xeb0 net/core/skbuff.c:1395 > Read of size 4294967282 at addr ffff8801d798009c by task syz-executor2/12942 > > kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412 > check_memory_region_inline mm/kasan/kasan.c:260 [inline] > check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267 > memcpy+0x23/0x50 mm/kasan/kasan.c:302 > memcpy include/linux/string.h:345 [inline] > skb_copy_from_linear_data include/linux/skbuff.h:3421 [inline] > __pskb_copy_fclone+0x2dd/0xeb0 net/core/skbuff.c:1395 > __pskb_copy include/linux/skbuff.h:1053 [inline] > pskb_copy include/linux/skbuff.h:2904 [inline] > skb_realloc_headroom+0xe7/0x120 net/core/skbuff.c:1539 > ipip6_tunnel_xmit net/ipv6/sit.c:965 [inline] > sit_tunnel_xmit+0xe1b/0x30d0 net/ipv6/sit.c:1029 > __netdev_start_xmit include/linux/netdevice.h:4325 [inline] > netdev_start_xmit include/linux/netdevice.h:4334 [inline] > xmit_one net/core/dev.c:3219 [inline] > dev_hard_start_xmit+0x295/0xc90 net/core/dev.c:3235 > __dev_queue_xmit+0x2f0d/0x3950 net/core/dev.c:3805 > dev_queue_xmit+0x17/0x20 net/core/dev.c:3838 > __bpf_tx_skb net/core/filter.c:2016 [inline] > __bpf_redirect_common net/core/filter.c:2054 [inline] > __bpf_redirect+0x5cf/0xb20 net/core/filter.c:2061 > ____bpf_clone_redirect net/core/filter.c:2094 [inline] > bpf_clone_redirect+0x2f6/0x490 net/core/filter.c:2066 > bpf_prog_41f2bcae09cd4ac3+0xb25/0x1000 > > The generated test constructs a packet with mac header, network > header, skb->data pointing to network header and skb->len 0. > > Redirecting to a sit0 through __bpf_redirect_no_mac pulls the > mac length, even though skb->data already is at skb->network_header. > bpf_prog_test_run_skb has already pulled it as LWT_XMIT !is_l2. > > Update the offset calculation to pull only if skb->data differs > from skb->network_header, which is not true in this case. > > The test itself can be run only from commit 1cf1cae963c2 ("bpf: > introduce BPF_PROG_TEST_RUN command"), but the same type of packets > with skb at network header could already be built from lwt xmit hooks, > so this fix is more relevant to that commit. > > Also set the mac header on redirect from LWT_XMIT, as even after this > change to __bpf_redirect_no_mac that field is expected to be set, but > is not yet in ip_finish_output2. > > Fixes: 3a0af8fd61f9 ("bpf: BPF for lightweight tunnel infrastructure") > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> Applied, thanks!
diff --git a/net/core/filter.c b/net/core/filter.c index 2b3b436ef5457..3a3f6473f24d6 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2020,18 +2020,19 @@ static inline int __bpf_tx_skb(struct net_device *dev, struct sk_buff *skb) static int __bpf_redirect_no_mac(struct sk_buff *skb, struct net_device *dev, u32 flags) { - /* skb->mac_len is not set on normal egress */ - unsigned int mlen = skb->network_header - skb->mac_header; + unsigned int mlen = skb_network_offset(skb); - __skb_pull(skb, mlen); + if (mlen) { + __skb_pull(skb, mlen); - /* At ingress, the mac header has already been pulled once. - * At egress, skb_pospull_rcsum has to be done in case that - * the skb is originated from ingress (i.e. a forwarded skb) - * to ensure that rcsum starts at net header. - */ - if (!skb_at_tc_ingress(skb)) - skb_postpull_rcsum(skb, skb_mac_header(skb), mlen); + /* At ingress, the mac header has already been pulled once. + * At egress, skb_pospull_rcsum has to be done in case that + * the skb is originated from ingress (i.e. a forwarded skb) + * to ensure that rcsum starts at net header. + */ + if (!skb_at_tc_ingress(skb)) + skb_postpull_rcsum(skb, skb_mac_header(skb), mlen); + } skb_pop_mac_header(skb); skb_reset_mac_len(skb); return flags & BPF_F_INGRESS ? diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c index 3e85437f71060..a648568c5e8fe 100644 --- a/net/core/lwt_bpf.c +++ b/net/core/lwt_bpf.c @@ -63,6 +63,7 @@ static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt, lwt->name ? : "<unknown>"); ret = BPF_OK; } else { + skb_reset_mac_header(skb); ret = skb_do_redirect(skb); if (ret == 0) ret = BPF_REDIRECT;