net/ipv4: use-after-free in ipv4_datagram_support_cmsg

net/ipv4: use-after-free in ipv4_datagram_support_cmsg

[Andrey Konovalov]

Hi,

I've got the following error report while fuzzing the kernel with syzkaller.

On commit 39da7c509acff13fc8cb12ec1bb20337c988ed36 (4.11-rc6).

Unfortunately it's not reproducible.

==================================================================
BUG: KASAN: use-after-free in ipv4_datagram_support_cmsg
net/ipv4/ip_sockglue.c:500 [inline] at addr ffff880059be0128
BUG: KASAN: use-after-free in ip_recv_error+0xb37/0xed0
net/ipv4/ip_sockglue.c:553 at addr ffff880059be0128
Read of size 4 by task syz-executor5/22308
CPU: 0 PID: 22308 Comm: syz-executor5 Not tainted 4.11.0-rc6+ #213
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:16 [inline]
 dump_stack+0x292/0x398 lib/dump_stack.c:52
 kasan_object_err+0x1c/0x70 mm/kasan/report.c:164
 print_address_description mm/kasan/report.c:202 [inline]
 kasan_report_error mm/kasan/report.c:291 [inline]
 kasan_report+0x252/0x510 mm/kasan/report.c:347
 __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:367
 ipv4_datagram_support_cmsg net/ipv4/ip_sockglue.c:500 [inline]
 ip_recv_error+0xb37/0xed0 net/ipv4/ip_sockglue.c:553
 udp_recvmsg+0xe70/0x1370 net/ipv4/udp.c:1421
 inet_recvmsg+0x13e/0x600 net/ipv4/af_inet.c:793
 sock_recvmsg_nosec net/socket.c:751 [inline]
 sock_recvmsg+0xd7/0x110 net/socket.c:758
 ___sys_recvmsg+0x2f4/0x730 net/socket.c:2156
 __sys_recvmsg+0x135/0x320 net/socket.c:2201
 SYSC_recvmsg net/socket.c:2213 [inline]
 SyS_recvmsg+0x2d/0x50 net/socket.c:2208
 entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x4458d9
RSP: 002b:00007f230ab31b58 EFLAGS: 00000286 ORIG_RAX: 000000000000002f
RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00000000004458d9
RDX: 0000000040002102 RSI: 0000000020edffc8 RDI: 0000000000000005
RBP: 00000000006e2d00 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000286 R12: 0000000000708000
R13: 0000000000000002 R14: 0000000000708008 R15: 00007f230ab32700
Object at ffff880059be0008, in cache kmalloc-8192 size: 8192
Allocated:
PID = 16445
 save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
 save_stack+0x43/0xd0 mm/kasan/kasan.c:513
 set_track mm/kasan/kasan.c:525 [inline]
 kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:616
 __kmalloc+0xa0/0x2d0 mm/slub.c:3745
 kzalloc include/linux/slab.h:495 [inline]
 alloc_netdev_mqs+0xbc1/0xf40 net/core/dev.c:7706
 br_add_bridge+0x34/0xd0 net/bridge/br_if.c:384
 br_ioctl_deviceless_stub+0x7fc/0xa30 net/bridge/br_ioctl.c:378
 sock_ioctl+0x256/0x440 net/socket.c:971
 vfs_ioctl fs/ioctl.c:45 [inline]
 do_vfs_ioctl+0x1bf/0x1780 fs/ioctl.c:685
 SYSC_ioctl fs/ioctl.c:700 [inline]
 SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
 entry_SYSCALL_64_fastpath+0x1f/0xc2
Freed:
PID = 22308
 save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
 save_stack+0x43/0xd0 mm/kasan/kasan.c:513
 set_track mm/kasan/kasan.c:525 [inline]
 kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:589
 slab_free_hook mm/slub.c:1357 [inline]
 slab_free_freelist_hook mm/slub.c:1379 [inline]
 slab_free mm/slub.c:2961 [inline]
 kfree+0xe8/0x2b0 mm/slub.c:3882
 kvfree+0x36/0x60 mm/util.c:337
 netdev_freemem+0x4c/0x60 net/core/dev.c:7658
 netdev_release+0x76/0x90 net/core/net-sysfs.c:1502
 device_release+0x18d/0x220 drivers/base/core.c:814
 kobject_cleanup lib/kobject.c:645 [inline]
 kobject_release+0xfa/0x1a0 lib/kobject.c:674
 kref_put include/linux/kref.h:72 [inline]
 kobject_put+0x6e/0xd0 lib/kobject.c:691
 netdev_run_todo+0x6b2/0xa40 net/core/dev.c:7563
 rtnl_unlock+0xe/0x10 net/core/rtnetlink.c:104
 br_del_bridge+0xb6/0xe0 net/bridge/br_if.c:422
 br_ioctl_deviceless_stub+0x324/0xa30 net/bridge/br_ioctl.c:380
 sock_ioctl+0x256/0x440 net/socket.c:971
 vfs_ioctl fs/ioctl.c:45 [inline]
 do_vfs_ioctl+0x1bf/0x1780 fs/ioctl.c:685
 SYSC_ioctl fs/ioctl.c:700 [inline]
 SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
 entry_SYSCALL_64_fastpath+0x1f/0xc2
Memory state around the buggy address:
 ffff880059be0000: fc fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff880059be0080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff880059be0100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                  ^
 ffff880059be0180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff880059be0200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

Re: net/ipv4: use-after-free in ipv4_datagram_support_cmsg

[Willem de Bruijn]

===================
> 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.

Re: net/ipv4: use-after-free in ipv4_datagram_support_cmsg

[Cong Wang]

On Wed, Apr 12, 2017 at 8:39 AM, Willem de Bruijn
<willemdebruijn.kernel@gmail.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?

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index ebd953b..a2aef45 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -497,7 +497,7 @@ static bool ipv4_datagram_support_cmsg(const
struct sock *sk,

        info = PKTINFO_SKB_CB(skb);
        info->ipi_spec_dst.s_addr = ip_hdr(skb)->saddr;
-       info->ipi_ifindex = skb->dev->ifindex;
+       info->ipi_ifindex = skb->skb_iif;
        return true;
 }

Re: net/ipv4: use-after-free in ipv4_datagram_support_cmsg

[Eric Dumazet]

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@gmail.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?
> 
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index ebd953b..a2aef45 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -497,7 +497,7 @@ static bool ipv4_datagram_support_cmsg(const
> struct sock *sk,
> 
>         info = PKTINFO_SKB_CB(skb);
>         info->ipi_spec_dst.s_addr = ip_hdr(skb)->saddr;
> -       info->ipi_ifindex = skb->dev->ifindex;
> +       info->ipi_ifindex = skb->skb_iif;
>         return true;
>  }

This seems to differ from the code in ipv4_pktinfo_prepare()

We probably want to unify things a bit.

                /* skb->cb is overloaded: prior to this point it is IP{6}CB
                 * which has interface index (iif) as the first member of the
                 * underlying inet{6}_skb_parm struct. This code then overlays
                 * PKTINFO_SKB_CB and in_pktinfo also has iif as the first
                 * element so the iif is picked up from the prior IPCB. If iif
                 * is the loopback interface, then return the sending interface
                 * (e.g., process binds socket to eth0 for Tx which is
                 * redirected to loopback in the rtable/dst).
                 */
                if (pktinfo->ipi_ifindex == LOOPBACK_IFINDEX)
                        pktinfo->ipi_ifindex = inet_iif(skb);

Re: net/ipv4: use-after-free in ipv4_datagram_support_cmsg

[Willem de Bruijn]

On Wed, Apr 12, 2017 at 4:47 PM, Eric Dumazet <eric.dumazet@gmail.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@gmail.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.

Re: net/ipv4: use-after-free in ipv4_datagram_support_cmsg

[Willem de Bruijn]

On Wed, Apr 12, 2017 at 6:25 PM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> On Wed, Apr 12, 2017 at 4:47 PM, Eric Dumazet <eric.dumazet@gmail.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@gmail.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