Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, May 9, 2018 at 5:05 PM, Willem de Bruijnwrote: > On Wed, May 9, 2018 at 3:36 PM, Eric Dumazet 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, ); 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_iter, len);
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, May 9, 2018 at 5:05 PM, Willem de Bruijn wrote: > On Wed, May 9, 2018 at 3:36 PM, Eric Dumazet 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, ); 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_iter, len);
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, May 9, 2018 at 3:36 PM, Eric Dumazetwrote: > > > 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.. >> > > I believe the bug happens if the sock_wmalloc() call at line 1921 has to > sleep. > > device can change (or at lest dev->hard_header_len can change) > > So we need to bailout if reserved/hhlen had changed. > > Or revert some patches, since dev_hold() and dev_put() are no longer high > cost, > since it is now using per cpu counter. Oh nice, another bug :/ That seems quite plausible. This reproducer does not modify hard_header_len, however. It sends a long array of zero byte requests with sendmmsg to eventually exceed so_rcvbuf of the error queue. Hard header length is 116 throughout.
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, May 9, 2018 at 3:36 PM, Eric Dumazet 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.. >> > > I believe the bug happens if the sock_wmalloc() call at line 1921 has to > sleep. > > device can change (or at lest dev->hard_header_len can change) > > So we need to bailout if reserved/hhlen had changed. > > Or revert some patches, since dev_hold() and dev_put() are no longer high > cost, > since it is now using per cpu counter. Oh nice, another bug :/ That seems quite plausible. This reproducer does not modify hard_header_len, however. It sends a long array of zero byte requests with sendmmsg to eventually exceed so_rcvbuf of the error queue. Hard header length is 116 throughout.
Re: KASAN: use-after-free Read in __dev_queue_xmit
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.. > I believe the bug happens if the sock_wmalloc() call at line 1921 has to sleep. device can change (or at lest dev->hard_header_len can change) So we need to bailout if reserved/hhlen had changed. Or revert some patches, since dev_hold() and dev_put() are no longer high cost, since it is now using per cpu counter.
Re: KASAN: use-after-free Read in __dev_queue_xmit
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.. > I believe the bug happens if the sock_wmalloc() call at line 1921 has to sleep. device can change (or at lest dev->hard_header_len can change) So we need to bailout if reserved/hhlen had changed. Or revert some patches, since dev_hold() and dev_put() are no longer high cost, since it is now using per cpu counter.
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, May 9, 2018 at 12:38 PM, Willem de Bruijnwrote: >>> But a crash with the same signature is still occurring, so it should >>> eventually >>> get reported again. C reproducer is here, it works on Linus' tree (commit >>> 036db8bd963): https://syzkaller.appspot.com/text?tag=ReproC=105b1ae780 >> >> This appears to be a separate issue. >> >> This reproducer requires a setsockopt SOL_SOCKET/SO_TIMESTAMPING >> to trigger the use-after-free. And the freed path also points at a >> timestamping >> skb: >> >> [ 31.963619] Freed by task 2672: >> [ 31.964006] __kasan_slab_free+0x125/0x170 >> [ 31.964509] kfree+0x8b/0x1a0 >> [ 31.964875] skb_free_head+0x6f/0xa0 >> [ 31.965314] skb_release_data+0x420/0x5a0 >> [ 31.965802] skb_release_all+0x46/0x60 >> [ 31.966260] kfree_skb+0x91/0x1c0 >> [ 31.99] __skb_complete_tx_timestamp+0x2e9/0x3d0 >> [ 31.967273] __skb_tstamp_tx+0x3b3/0x620 >> [ 31.967774] __dev_queue_xmit+0xed5/0x1a20 >> [ 31.968300] packet_sendmsg+0x36fd/0x5400 >> [ 31.968821] sock_sendmsg+0xc0/0x100 >> [ 31.969284] ___sys_sendmsg+0x367/0x880 >> [ 31.969777] __sys_sendmmsg+0x178/0x410 >> [ 31.970267] __x64_sys_sendmmsg+0x99/0x100 >> [ 31.970789] do_syscall_64+0x9a/0x2c0 >> [ 31.971260] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > This is a rare path taken when the timestamp skb cannot be queued > onto the socket (likely because of insufficient rcvbuf). > > Somehow, freeing the timestamp skb triggers this use-after-free in > the original skb from which the timestamp was cloned. As if there > is a bug in the shared info dataref. 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..
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, May 9, 2018 at 12:38 PM, Willem de Bruijn wrote: >>> But a crash with the same signature is still occurring, so it should >>> eventually >>> get reported again. C reproducer is here, it works on Linus' tree (commit >>> 036db8bd963): https://syzkaller.appspot.com/text?tag=ReproC=105b1ae780 >> >> This appears to be a separate issue. >> >> This reproducer requires a setsockopt SOL_SOCKET/SO_TIMESTAMPING >> to trigger the use-after-free. And the freed path also points at a >> timestamping >> skb: >> >> [ 31.963619] Freed by task 2672: >> [ 31.964006] __kasan_slab_free+0x125/0x170 >> [ 31.964509] kfree+0x8b/0x1a0 >> [ 31.964875] skb_free_head+0x6f/0xa0 >> [ 31.965314] skb_release_data+0x420/0x5a0 >> [ 31.965802] skb_release_all+0x46/0x60 >> [ 31.966260] kfree_skb+0x91/0x1c0 >> [ 31.99] __skb_complete_tx_timestamp+0x2e9/0x3d0 >> [ 31.967273] __skb_tstamp_tx+0x3b3/0x620 >> [ 31.967774] __dev_queue_xmit+0xed5/0x1a20 >> [ 31.968300] packet_sendmsg+0x36fd/0x5400 >> [ 31.968821] sock_sendmsg+0xc0/0x100 >> [ 31.969284] ___sys_sendmsg+0x367/0x880 >> [ 31.969777] __sys_sendmmsg+0x178/0x410 >> [ 31.970267] __x64_sys_sendmmsg+0x99/0x100 >> [ 31.970789] do_syscall_64+0x9a/0x2c0 >> [ 31.971260] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > This is a rare path taken when the timestamp skb cannot be queued > onto the socket (likely because of insufficient rcvbuf). > > Somehow, freeing the timestamp skb triggers this use-after-free in > the original skb from which the timestamp was cloned. As if there > is a bug in the shared info dataref. 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..
Re: KASAN: use-after-free Read in __dev_queue_xmit
>> But a crash with the same signature is still occurring, so it should >> eventually >> get reported again. C reproducer is here, it works on Linus' tree (commit >> 036db8bd963): https://syzkaller.appspot.com/text?tag=ReproC=105b1ae780 > > This appears to be a separate issue. > > This reproducer requires a setsockopt SOL_SOCKET/SO_TIMESTAMPING > to trigger the use-after-free. And the freed path also points at a > timestamping > skb: > > [ 31.963619] Freed by task 2672: > [ 31.964006] __kasan_slab_free+0x125/0x170 > [ 31.964509] kfree+0x8b/0x1a0 > [ 31.964875] skb_free_head+0x6f/0xa0 > [ 31.965314] skb_release_data+0x420/0x5a0 > [ 31.965802] skb_release_all+0x46/0x60 > [ 31.966260] kfree_skb+0x91/0x1c0 > [ 31.99] __skb_complete_tx_timestamp+0x2e9/0x3d0 > [ 31.967273] __skb_tstamp_tx+0x3b3/0x620 > [ 31.967774] __dev_queue_xmit+0xed5/0x1a20 > [ 31.968300] packet_sendmsg+0x36fd/0x5400 > [ 31.968821] sock_sendmsg+0xc0/0x100 > [ 31.969284] ___sys_sendmsg+0x367/0x880 > [ 31.969777] __sys_sendmmsg+0x178/0x410 > [ 31.970267] __x64_sys_sendmmsg+0x99/0x100 > [ 31.970789] do_syscall_64+0x9a/0x2c0 > [ 31.971260] entry_SYSCALL_64_after_hwframe+0x44/0xa9 This is a rare path taken when the timestamp skb cannot be queued onto the socket (likely because of insufficient rcvbuf). Somehow, freeing the timestamp skb triggers this use-after-free in the original skb from which the timestamp was cloned. As if there is a bug in the shared info dataref. The report does occur on reading a shinfo field (gso_size).
Re: KASAN: use-after-free Read in __dev_queue_xmit
>> But a crash with the same signature is still occurring, so it should >> eventually >> get reported again. C reproducer is here, it works on Linus' tree (commit >> 036db8bd963): https://syzkaller.appspot.com/text?tag=ReproC=105b1ae780 > > This appears to be a separate issue. > > This reproducer requires a setsockopt SOL_SOCKET/SO_TIMESTAMPING > to trigger the use-after-free. And the freed path also points at a > timestamping > skb: > > [ 31.963619] Freed by task 2672: > [ 31.964006] __kasan_slab_free+0x125/0x170 > [ 31.964509] kfree+0x8b/0x1a0 > [ 31.964875] skb_free_head+0x6f/0xa0 > [ 31.965314] skb_release_data+0x420/0x5a0 > [ 31.965802] skb_release_all+0x46/0x60 > [ 31.966260] kfree_skb+0x91/0x1c0 > [ 31.99] __skb_complete_tx_timestamp+0x2e9/0x3d0 > [ 31.967273] __skb_tstamp_tx+0x3b3/0x620 > [ 31.967774] __dev_queue_xmit+0xed5/0x1a20 > [ 31.968300] packet_sendmsg+0x36fd/0x5400 > [ 31.968821] sock_sendmsg+0xc0/0x100 > [ 31.969284] ___sys_sendmsg+0x367/0x880 > [ 31.969777] __sys_sendmmsg+0x178/0x410 > [ 31.970267] __x64_sys_sendmmsg+0x99/0x100 > [ 31.970789] do_syscall_64+0x9a/0x2c0 > [ 31.971260] entry_SYSCALL_64_after_hwframe+0x44/0xa9 This is a rare path taken when the timestamp skb cannot be queued onto the socket (likely because of insufficient rcvbuf). Somehow, freeing the timestamp skb triggers this use-after-free in the original skb from which the timestamp was cloned. As if there is a bug in the shared info dataref. The report does occur on reading a shinfo field (gso_size).
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, May 9, 2018 at 3:37 AM, Eric Biggerswrote: > On Wed, Jan 03, 2018 at 10:53:14PM -0800, Eric Dumazet wrote: >> On Wed, 2018-01-03 at 21:13 -0800, Eric Dumazet wrote: >> > Note: all commands must start from beginning of the line in the email body. >> > >> > I guess skb_probe_transport_header() should be hardened to reject malicious >> > packets given by user space, instead of being gentle. >> >> Although bug triggered for this particular repro is in flow dissector >> :/ >> >> I will test : >> >> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c >> index >> 15ce300637650e17fcab7e378b20fe7972686d46..544bddf08e13c7f6e47aadc737244c9ba5af56b2 >> 100644 >> --- a/net/core/flow_dissector.c >> +++ b/net/core/flow_dissector.c >> @@ -976,8 +976,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb, >> out_good: >> ret = true; >> >> - key_control->thoff = (u16)nhoff; >> out: >> + key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); >> key_basic->n_proto = proto; >> key_basic->ip_proto = ip_proto; >> >> @@ -985,7 +985,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb, >> >> out_bad: >> ret = false; >> - key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); >> goto out; >> } >> EXPORT_SYMBOL(__skb_flow_dissect); > > Fix for this was commit d0c081b49137cd: > > #syz fix: flow_dissector: properly cap thoff field > > But a crash with the same signature is still occurring, so it should > eventually > get reported again. C reproducer is here, it works on Linus' tree (commit > 036db8bd963): https://syzkaller.appspot.com/text?tag=ReproC=105b1ae780 This appears to be a separate issue. This reproducer requires a setsockopt SOL_SOCKET/SO_TIMESTAMPING to trigger the use-after-free. And the freed path also points at a timestamping skb: [ 31.963619] Freed by task 2672: [ 31.964006] __kasan_slab_free+0x125/0x170 [ 31.964509] kfree+0x8b/0x1a0 [ 31.964875] skb_free_head+0x6f/0xa0 [ 31.965314] skb_release_data+0x420/0x5a0 [ 31.965802] skb_release_all+0x46/0x60 [ 31.966260] kfree_skb+0x91/0x1c0 [ 31.99] __skb_complete_tx_timestamp+0x2e9/0x3d0 [ 31.967273] __skb_tstamp_tx+0x3b3/0x620 [ 31.967774] __dev_queue_xmit+0xed5/0x1a20 [ 31.968300] packet_sendmsg+0x36fd/0x5400 [ 31.968821] sock_sendmsg+0xc0/0x100 [ 31.969284] ___sys_sendmsg+0x367/0x880 [ 31.969777] __sys_sendmmsg+0x178/0x410 [ 31.970267] __x64_sys_sendmmsg+0x99/0x100 [ 31.970789] do_syscall_64+0x9a/0x2c0 [ 31.971260] entry_SYSCALL_64_after_hwframe+0x44/0xa9 The data skb itself is zero bytes, it appears.
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, May 9, 2018 at 3:37 AM, Eric Biggers wrote: > On Wed, Jan 03, 2018 at 10:53:14PM -0800, Eric Dumazet wrote: >> On Wed, 2018-01-03 at 21:13 -0800, Eric Dumazet wrote: >> > Note: all commands must start from beginning of the line in the email body. >> > >> > I guess skb_probe_transport_header() should be hardened to reject malicious >> > packets given by user space, instead of being gentle. >> >> Although bug triggered for this particular repro is in flow dissector >> :/ >> >> I will test : >> >> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c >> index >> 15ce300637650e17fcab7e378b20fe7972686d46..544bddf08e13c7f6e47aadc737244c9ba5af56b2 >> 100644 >> --- a/net/core/flow_dissector.c >> +++ b/net/core/flow_dissector.c >> @@ -976,8 +976,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb, >> out_good: >> ret = true; >> >> - key_control->thoff = (u16)nhoff; >> out: >> + key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); >> key_basic->n_proto = proto; >> key_basic->ip_proto = ip_proto; >> >> @@ -985,7 +985,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb, >> >> out_bad: >> ret = false; >> - key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); >> goto out; >> } >> EXPORT_SYMBOL(__skb_flow_dissect); > > Fix for this was commit d0c081b49137cd: > > #syz fix: flow_dissector: properly cap thoff field > > But a crash with the same signature is still occurring, so it should > eventually > get reported again. C reproducer is here, it works on Linus' tree (commit > 036db8bd963): https://syzkaller.appspot.com/text?tag=ReproC=105b1ae780 This appears to be a separate issue. This reproducer requires a setsockopt SOL_SOCKET/SO_TIMESTAMPING to trigger the use-after-free. And the freed path also points at a timestamping skb: [ 31.963619] Freed by task 2672: [ 31.964006] __kasan_slab_free+0x125/0x170 [ 31.964509] kfree+0x8b/0x1a0 [ 31.964875] skb_free_head+0x6f/0xa0 [ 31.965314] skb_release_data+0x420/0x5a0 [ 31.965802] skb_release_all+0x46/0x60 [ 31.966260] kfree_skb+0x91/0x1c0 [ 31.99] __skb_complete_tx_timestamp+0x2e9/0x3d0 [ 31.967273] __skb_tstamp_tx+0x3b3/0x620 [ 31.967774] __dev_queue_xmit+0xed5/0x1a20 [ 31.968300] packet_sendmsg+0x36fd/0x5400 [ 31.968821] sock_sendmsg+0xc0/0x100 [ 31.969284] ___sys_sendmsg+0x367/0x880 [ 31.969777] __sys_sendmmsg+0x178/0x410 [ 31.970267] __x64_sys_sendmmsg+0x99/0x100 [ 31.970789] do_syscall_64+0x9a/0x2c0 [ 31.971260] entry_SYSCALL_64_after_hwframe+0x44/0xa9 The data skb itself is zero bytes, it appears.
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, Jan 03, 2018 at 10:53:14PM -0800, Eric Dumazet wrote: > On Wed, 2018-01-03 at 21:13 -0800, Eric Dumazet wrote: > > Note: all commands must start from beginning of the line in the email body. > > > > I guess skb_probe_transport_header() should be hardened to reject malicious > > packets given by user space, instead of being gentle. > > Although bug triggered for this particular repro is in flow dissector > :/ > > I will test : > > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c > index > 15ce300637650e17fcab7e378b20fe7972686d46..544bddf08e13c7f6e47aadc737244c9ba5af56b2 > 100644 > --- a/net/core/flow_dissector.c > +++ b/net/core/flow_dissector.c > @@ -976,8 +976,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb, > out_good: > ret = true; > > - key_control->thoff = (u16)nhoff; > out: > + key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); > key_basic->n_proto = proto; > key_basic->ip_proto = ip_proto; > > @@ -985,7 +985,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb, > > out_bad: > ret = false; > - key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); > goto out; > } > EXPORT_SYMBOL(__skb_flow_dissect); Fix for this was commit d0c081b49137cd: #syz fix: flow_dissector: properly cap thoff field But a crash with the same signature is still occurring, so it should eventually get reported again. C reproducer is here, it works on Linus' tree (commit 036db8bd963): https://syzkaller.appspot.com/text?tag=ReproC=105b1ae780 - Eric
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, Jan 03, 2018 at 10:53:14PM -0800, Eric Dumazet wrote: > On Wed, 2018-01-03 at 21:13 -0800, Eric Dumazet wrote: > > Note: all commands must start from beginning of the line in the email body. > > > > I guess skb_probe_transport_header() should be hardened to reject malicious > > packets given by user space, instead of being gentle. > > Although bug triggered for this particular repro is in flow dissector > :/ > > I will test : > > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c > index > 15ce300637650e17fcab7e378b20fe7972686d46..544bddf08e13c7f6e47aadc737244c9ba5af56b2 > 100644 > --- a/net/core/flow_dissector.c > +++ b/net/core/flow_dissector.c > @@ -976,8 +976,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb, > out_good: > ret = true; > > - key_control->thoff = (u16)nhoff; > out: > + key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); > key_basic->n_proto = proto; > key_basic->ip_proto = ip_proto; > > @@ -985,7 +985,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb, > > out_bad: > ret = false; > - key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); > goto out; > } > EXPORT_SYMBOL(__skb_flow_dissect); Fix for this was commit d0c081b49137cd: #syz fix: flow_dissector: properly cap thoff field But a crash with the same signature is still occurring, so it should eventually get reported again. C reproducer is here, it works on Linus' tree (commit 036db8bd963): https://syzkaller.appspot.com/text?tag=ReproC=105b1ae780 - Eric
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, 2018-01-03 at 21:13 -0800, Eric Dumazet wrote: > Note: all commands must start from beginning of the line in the email body. > > I guess skb_probe_transport_header() should be hardened to reject malicious > packets given by user space, instead of being gentle. Although bug triggered for this particular repro is in flow dissector :/ I will test : diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c index 15ce300637650e17fcab7e378b20fe7972686d46..544bddf08e13c7f6e47aadc737244c9ba5af56b2 100644 --- a/net/core/flow_dissector.c +++ b/net/core/flow_dissector.c @@ -976,8 +976,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb, out_good: ret = true; - key_control->thoff = (u16)nhoff; out: + key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); key_basic->n_proto = proto; key_basic->ip_proto = ip_proto; @@ -985,7 +985,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb, out_bad: ret = false; - key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); goto out; } EXPORT_SYMBOL(__skb_flow_dissect);
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, 2018-01-03 at 21:13 -0800, Eric Dumazet wrote: > Note: all commands must start from beginning of the line in the email body. > > I guess skb_probe_transport_header() should be hardened to reject malicious > packets given by user space, instead of being gentle. Although bug triggered for this particular repro is in flow dissector :/ I will test : diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c index 15ce300637650e17fcab7e378b20fe7972686d46..544bddf08e13c7f6e47aadc737244c9ba5af56b2 100644 --- a/net/core/flow_dissector.c +++ b/net/core/flow_dissector.c @@ -976,8 +976,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb, out_good: ret = true; - key_control->thoff = (u16)nhoff; out: + key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); key_basic->n_proto = proto; key_basic->ip_proto = ip_proto; @@ -985,7 +985,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb, out_bad: ret = false; - key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen); goto out; } EXPORT_SYMBOL(__skb_flow_dissect);
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, Jan 3, 2018 at 8:58 PM, syzbotwrote: > Hello, > > syzkaller hit the following crash on > 37759fa6d0fa9e4d6036d19ac12f555bfc0aeafd > git://git.cmpxchg.org/linux-mmots.git/master > compiler: gcc (GCC) 7.1.1 20170620 > .config is attached > Raw console output is attached. > C reproducer is attached > syzkaller reproducer is attached. See https://goo.gl/kgGztJ > for information about syzkaller reproducers > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+71d74a5406d02057d...@syzkaller.appspotmail.com > It will help syzbot understand when the bug is fixed. See footer for > details. > If you forward the report, please keep this part and the footer. > > == > BUG: KASAN: use-after-free in __tcp_hdrlen include/linux/tcp.h:35 [inline] > BUG: KASAN: use-after-free in tcp_hdrlen include/linux/tcp.h:40 [inline] > BUG: KASAN: use-after-free in qdisc_pkt_len_init net/core/dev.c:3160 > [inline] > BUG: KASAN: use-after-free in __dev_queue_xmit+0x20d3/0x2200 > net/core/dev.c:3465 > Read of size 2 at addr 8801cb360d38 by task syzkaller769484/3147 > > CPU: 0 PID: 3147 Comm: syzkaller769484 Not tainted 4.15.0-rc4-mm1+ #49 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:17 [inline] > dump_stack+0x194/0x257 lib/dump_stack.c:53 > print_address_description+0x73/0x250 mm/kasan/report.c:256 > kasan_report_error mm/kasan/report.c:355 [inline] > kasan_report+0x23b/0x360 mm/kasan/report.c:413 > __asan_report_load2_noabort+0x14/0x20 mm/kasan/report.c:432 > __tcp_hdrlen include/linux/tcp.h:35 [inline] > tcp_hdrlen include/linux/tcp.h:40 [inline] > qdisc_pkt_len_init net/core/dev.c:3160 [inline] > __dev_queue_xmit+0x20d3/0x2200 net/core/dev.c:3465 > dev_queue_xmit+0x17/0x20 net/core/dev.c:3554 > packet_snd net/packet/af_packet.c:2943 [inline] > packet_sendmsg+0x3ad5/0x60a0 net/packet/af_packet.c:2968 > sock_sendmsg_nosec net/socket.c:628 [inline] > sock_sendmsg+0xca/0x110 net/socket.c:638 > sock_write_iter+0x31a/0x5d0 net/socket.c:907 > call_write_iter include/linux/fs.h:1776 [inline] > new_sync_write fs/read_write.c:469 [inline] > __vfs_write+0x684/0x970 fs/read_write.c:482 > vfs_write+0x189/0x510 fs/read_write.c:544 > SYSC_write fs/read_write.c:589 [inline] > SyS_write+0xef/0x220 fs/read_write.c:581 > entry_SYSCALL_64_fastpath+0x1f/0x96 > RIP: 0033:0x444b89 > RSP: 002b:007eff78 EFLAGS: 0293 ORIG_RAX: 0001 > RAX: ffda RBX: 7fff91d02bb0 RCX: 00444b89 > RDX: 005d RSI: 20384000 RDI: 0005 > RBP: R08: 000120080522 R09: 000120080522 > R10: 000120080522 R11: 0293 R12: 00402780 > R13: 00402810 R14: R15: > > Allocated by task 1643: > save_stack+0x43/0xd0 mm/kasan/kasan.c:447 > set_track mm/kasan/kasan.c:459 [inline] > kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551 > kmem_cache_alloc_trace+0x136/0x750 mm/slab.c:3607 > kmalloc include/linux/slab.h:516 [inline] > kzalloc include/linux/slab.h:705 [inline] > kernfs_fop_open+0x357/0xeb0 fs/kernfs/file.c:648 > do_dentry_open+0x667/0xd40 fs/open.c:752 > vfs_open+0x107/0x220 fs/open.c:866 > do_last fs/namei.c:3397 [inline] > path_openat+0x1151/0x3530 fs/namei.c:3537 > do_filp_open+0x25b/0x3b0 fs/namei.c:3572 > do_sys_open+0x502/0x6d0 fs/open.c:1059 > SYSC_open fs/open.c:1077 [inline] > SyS_open+0x2d/0x40 fs/open.c:1072 > entry_SYSCALL_64_fastpath+0x1f/0x96 > > Freed by task 1643: > save_stack+0x43/0xd0 mm/kasan/kasan.c:447 > set_track mm/kasan/kasan.c:459 [inline] > kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524 > __cache_free mm/slab.c:3485 [inline] > kfree+0xd6/0x260 mm/slab.c:3800 > kernfs_fop_release+0x13f/0x180 fs/kernfs/file.c:783 > __fput+0x327/0x7e0 fs/file_table.c:209 > fput+0x15/0x20 fs/file_table.c:243 > task_work_run+0x199/0x270 kernel/task_work.c:113 > tracehook_notify_resume include/linux/tracehook.h:191 [inline] > exit_to_usermode_loop+0x275/0x2f0 arch/x86/entry/common.c:165 > prepare_exit_to_usermode arch/x86/entry/common.c:195 [inline] > syscall_return_slowpath+0x490/0x550 arch/x86/entry/common.c:264 > entry_SYSCALL_64_fastpath+0x94/0x96 > > The buggy address belongs to the object at 8801cb360cc0 > which belongs to the cache kmalloc-512 of size 512 > The buggy address is located 120 bytes inside of > 512-byte region [8801cb360cc0, 8801cb360ec0) > The buggy address belongs to the page: > page:ea00072cd800 count:1 mapcount:0 mapping:8801cb360040 index:0x0 > flags: 0x2fffc000100(slab) > raw: 02fffc000100 8801cb360040 00010006 > raw: ea00072bb6a0 ea00072bf720 8801dac00940 > page dumped because: kasan: bad
Re: KASAN: use-after-free Read in __dev_queue_xmit
On Wed, Jan 3, 2018 at 8:58 PM, syzbot wrote: > Hello, > > syzkaller hit the following crash on > 37759fa6d0fa9e4d6036d19ac12f555bfc0aeafd > git://git.cmpxchg.org/linux-mmots.git/master > compiler: gcc (GCC) 7.1.1 20170620 > .config is attached > Raw console output is attached. > C reproducer is attached > syzkaller reproducer is attached. See https://goo.gl/kgGztJ > for information about syzkaller reproducers > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+71d74a5406d02057d...@syzkaller.appspotmail.com > It will help syzbot understand when the bug is fixed. See footer for > details. > If you forward the report, please keep this part and the footer. > > == > BUG: KASAN: use-after-free in __tcp_hdrlen include/linux/tcp.h:35 [inline] > BUG: KASAN: use-after-free in tcp_hdrlen include/linux/tcp.h:40 [inline] > BUG: KASAN: use-after-free in qdisc_pkt_len_init net/core/dev.c:3160 > [inline] > BUG: KASAN: use-after-free in __dev_queue_xmit+0x20d3/0x2200 > net/core/dev.c:3465 > Read of size 2 at addr 8801cb360d38 by task syzkaller769484/3147 > > CPU: 0 PID: 3147 Comm: syzkaller769484 Not tainted 4.15.0-rc4-mm1+ #49 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:17 [inline] > dump_stack+0x194/0x257 lib/dump_stack.c:53 > print_address_description+0x73/0x250 mm/kasan/report.c:256 > kasan_report_error mm/kasan/report.c:355 [inline] > kasan_report+0x23b/0x360 mm/kasan/report.c:413 > __asan_report_load2_noabort+0x14/0x20 mm/kasan/report.c:432 > __tcp_hdrlen include/linux/tcp.h:35 [inline] > tcp_hdrlen include/linux/tcp.h:40 [inline] > qdisc_pkt_len_init net/core/dev.c:3160 [inline] > __dev_queue_xmit+0x20d3/0x2200 net/core/dev.c:3465 > dev_queue_xmit+0x17/0x20 net/core/dev.c:3554 > packet_snd net/packet/af_packet.c:2943 [inline] > packet_sendmsg+0x3ad5/0x60a0 net/packet/af_packet.c:2968 > sock_sendmsg_nosec net/socket.c:628 [inline] > sock_sendmsg+0xca/0x110 net/socket.c:638 > sock_write_iter+0x31a/0x5d0 net/socket.c:907 > call_write_iter include/linux/fs.h:1776 [inline] > new_sync_write fs/read_write.c:469 [inline] > __vfs_write+0x684/0x970 fs/read_write.c:482 > vfs_write+0x189/0x510 fs/read_write.c:544 > SYSC_write fs/read_write.c:589 [inline] > SyS_write+0xef/0x220 fs/read_write.c:581 > entry_SYSCALL_64_fastpath+0x1f/0x96 > RIP: 0033:0x444b89 > RSP: 002b:007eff78 EFLAGS: 0293 ORIG_RAX: 0001 > RAX: ffda RBX: 7fff91d02bb0 RCX: 00444b89 > RDX: 005d RSI: 20384000 RDI: 0005 > RBP: R08: 000120080522 R09: 000120080522 > R10: 000120080522 R11: 0293 R12: 00402780 > R13: 00402810 R14: R15: > > Allocated by task 1643: > save_stack+0x43/0xd0 mm/kasan/kasan.c:447 > set_track mm/kasan/kasan.c:459 [inline] > kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551 > kmem_cache_alloc_trace+0x136/0x750 mm/slab.c:3607 > kmalloc include/linux/slab.h:516 [inline] > kzalloc include/linux/slab.h:705 [inline] > kernfs_fop_open+0x357/0xeb0 fs/kernfs/file.c:648 > do_dentry_open+0x667/0xd40 fs/open.c:752 > vfs_open+0x107/0x220 fs/open.c:866 > do_last fs/namei.c:3397 [inline] > path_openat+0x1151/0x3530 fs/namei.c:3537 > do_filp_open+0x25b/0x3b0 fs/namei.c:3572 > do_sys_open+0x502/0x6d0 fs/open.c:1059 > SYSC_open fs/open.c:1077 [inline] > SyS_open+0x2d/0x40 fs/open.c:1072 > entry_SYSCALL_64_fastpath+0x1f/0x96 > > Freed by task 1643: > save_stack+0x43/0xd0 mm/kasan/kasan.c:447 > set_track mm/kasan/kasan.c:459 [inline] > kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524 > __cache_free mm/slab.c:3485 [inline] > kfree+0xd6/0x260 mm/slab.c:3800 > kernfs_fop_release+0x13f/0x180 fs/kernfs/file.c:783 > __fput+0x327/0x7e0 fs/file_table.c:209 > fput+0x15/0x20 fs/file_table.c:243 > task_work_run+0x199/0x270 kernel/task_work.c:113 > tracehook_notify_resume include/linux/tracehook.h:191 [inline] > exit_to_usermode_loop+0x275/0x2f0 arch/x86/entry/common.c:165 > prepare_exit_to_usermode arch/x86/entry/common.c:195 [inline] > syscall_return_slowpath+0x490/0x550 arch/x86/entry/common.c:264 > entry_SYSCALL_64_fastpath+0x94/0x96 > > The buggy address belongs to the object at 8801cb360cc0 > which belongs to the cache kmalloc-512 of size 512 > The buggy address is located 120 bytes inside of > 512-byte region [8801cb360cc0, 8801cb360ec0) > The buggy address belongs to the page: > page:ea00072cd800 count:1 mapcount:0 mapping:8801cb360040 index:0x0 > flags: 0x2fffc000100(slab) > raw: 02fffc000100 8801cb360040 00010006 > raw: ea00072bb6a0 ea00072bf720 8801dac00940 > page dumped because: kasan: bad access detected > > Memory state around the buggy address: >