netdev - Re: Bug URGENT Report with new kernel 5.5.10-5.6-rc6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 Mar 2020 09:40:59 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Florian Westphal <fw@...len.de>,
        Pablo Neira Ayuso <pablo@...filter.org>
Cc:     Martin Zaharinov <micron10@...il.com>,
        netfilter-devel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: Bug URGENT Report with new kernel 5.5.10-5.6-rc6



On 3/19/20 3:52 AM, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@...filter.org> wrote:
>> On Thu, Mar 19, 2020 at 11:34:38AM +0100, Florian Westphal wrote:
>>> Martin Zaharinov <micron10@...il.com> wrote:
>>>
>>> [ trimming CC ]
>>>
>>> Please revert
>>>
>>> commit 28f8bfd1ac948403ebd5c8070ae1e25421560059
>>> netfilter: Support iif matches in POSTROUTING
>>
>> Please, specify a short description to append to the revert.
> 
> TCP makes use of the rb_node in sk_buff for its retransmit queue,
> amongst others.


Only for master skbs kept in TCP internal queues (rtx rb tree)

However the packets leaving TCP stack are clones.

  skb->dev aliases to this storage, i.e., passing
> skb->dev as the input interface in postrouting may point to another
> sk_buff instead.
> This will cause crashes and data corruption with nf_queue, as we will
> attempt to increment a random pcpu variable when calling dev_hold().
> 
> Also, the memory address may also be free'd, which gives UAF splat.
> 

This seems to suggest clones skb->dev should be cleared before leaving TCP stack,
if some layer is confused because skb->dev has not yet been set by IP layer ?

Untested patch :

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 306e25d743e8de1bfe23d6e3b3a9fb0f23664912..c40fb3880307aa3156d01a8b49f1296657346cfd 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1228,6 +1228,7 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
        /* Cleanup our debris for IP stacks */
        memset(skb->cb, 0, max(sizeof(struct inet_skb_parm),
                               sizeof(struct inet6_skb_parm)));
+       skb->dev = NULL;
 
        tcp_add_tx_delay(skb, tp);