Messages in this thread | | | From | Willem de Bruijn <> | Date | Thu, 10 May 2018 17:49:08 -0400 | Subject | Re: KASAN: use-after-free Read in __dev_queue_xmit |
| |
On Wed, May 9, 2018 at 5:05 PM, Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > On Wed, May 9, 2018 at 3:36 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: >> >> >> On 05/09/2018 12:21 PM, Willem de Bruijn wrote: >> >>> Indeed. The skb shared info struct is zeroed by dev_validate_header >>> as a result of dev->hard_header_len exceeding skb->end - skb->data. >>> >>> Not exactly sure yet how this can happen. The hard header length space >>> is accounted for during allocation as reserved memory. But, >>> packet_alloc_skb does call skb_reserve(), moving skb->data >>> effectively beyond this reserved region. >>> >>> It may be incorrect to pass skb->data to dev_validate_header, as that >>> does not point to the start of the ll_header anymore. Still figuring out what >>> the right fix is..
The following resolves the issue.
packet_alloc_skb already calls skb_reserve(skb, reserve), so now the network header should start at 0, not at reserve.
If SOCK_DGRAM, dev_hard_header() calls skb_push for the link layer and returns this offset.
If SOCK_RAW, we should do the same and use the reserved space to write the link layer.
Now behavior is the same as in tpacket_snd.
@@ -2898,19 +2911,26 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) tlen = dev->needed_tailroom; linear = __virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len); linear = max(linear, min_t(int, len, dev->hard_header_len)); skb = packet_alloc_skb(sk, hlen + tlen, hlen, len, linear, msg->msg_flags & MSG_DONTWAIT, &err); if (skb == NULL) goto out_unlock;
- skb_set_network_header(skb, reserve); + skb_reset_network_header(skb);
err = -EINVAL; if (sock->type == SOCK_DGRAM) { offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len); if (unlikely(offset < 0)) goto out_free; + } else { + skb_push(skb, dev->hard_header_len); }
/* Returns -EFAULT on error */ err = skb_copy_datagram_from_iter(skb, offset, &msg->msg_iter, len);
| |