[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAF=yD-KL5i_567+fUjny=0VWwDRFb0mh0UE-9ahZFRB__i=brA@mail.gmail.com>
Date: Wed, 12 Apr 2017 19:26:22 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Eric Dumazet <eric.dumazet@...il.com>
Cc: Cong Wang <xiyou.wangcong@...il.com>,
Andrey Konovalov <andreyknvl@...gle.com>,
"David S. Miller" <davem@...emloft.net>,
Alexey Kuznetsov <kuznet@....inr.ac.ru>,
James Morris <jmorris@...ei.org>,
Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
Patrick McHardy <kaber@...sh.net>,
netdev <netdev@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Eric Dumazet <edumazet@...gle.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
Kostya Serebryany <kcc@...gle.com>,
syzkaller <syzkaller@...glegroups.com>
Subject: Re: net/ipv4: use-after-free in ipv4_datagram_support_cmsg
On Wed, Apr 12, 2017 at 6:25 PM, Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
> On Wed, Apr 12, 2017 at 4:47 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
>> On Wed, 2017-04-12 at 13:07 -0700, Cong Wang wrote:
>>> On Wed, Apr 12, 2017 at 8:39 AM, Willem de Bruijn
>>> <willemdebruijn.kernel@...il.com> wrote:
>>> > ===================
>>> >> BUG: KASAN: use-after-free in ipv4_datagram_support_cmsg
>>> >> net/ipv4/ip_sockglue.c:500 [inline] at addr ffff880059be0128
>>> >
>>> > Thanks for the report. This is accessing skb->dev from within recvmsg() at line
>>> >
>>> > info->ipi_ifindex = skb->dev->ifindex;
>>> >
>>> > Introduced in 829ae9d61165 ("net-timestamp: allow reading recv cmsg on
>>> > errqueue with origin tstamp"). At this time the device may indeed have
>>> > gone away. I'm having a look at a way to read this in the receive BH
>>> > and store the ifindex.
>>>
>>> Why not use skb_iif?
>
> This code is called from the error path for transmit timestamps.
>
> We can make use of the fact that SKB_EXT_ERR used on enqueue has iif as
> the first field in its control block. This also holds for the PKTINFO_SKB_CB
> struct to which skb->cb is cast on dequeue when it copies pktinfo to userspace.
> So if set on enqueue in __skb_complete_tx_timestamp, no conversion operation
> is even needed on dequeue, let alone the currently buggy line that touches
> skb->dev.
>
> This iif cast was added for this purpose in the receive path in 0b922b7a829c
> ("net: original ingress device index in PKTINFO").
>
> The device pointer is valid on enqueue for all paths called from device drivers,
> as well as from dev_queue_xmit for SCM_TSTAMP_SCHED generation in
> __dev_queue_xmit. The exception is SCM_TSTAMP_ACK generation, but
> there skb->dev is NULL.
>
> The v6 path does need a conversion, but already does this in
> ip6_datagram_recv_common_ctl. There, too, we can remove the buggy
> logic to set it from skb->dev->ifindex in ip6_datagram_support_cmsg.
>
> I will send a patch.
Sent http://patchwork.ozlabs.org/patch/750197
Powered by blists - more mailing lists