26 Sep, 2018

1 commit

  • [ Upstream commit 934ffce1343f22ed5e2d0bd6da4440f4848074de ]

    Fix a static code checker warning:

    net/xfrm/xfrm_policy.c:1836 xfrm_resolve_and_create_bundle() warn: passing zero to 'ERR_PTR'

    xfrm_tmpl_resolve return 0 just means no xdst found, return NULL
    instead of passing zero to ERR_PTR.

    Fixes: d809ec895505 ("xfrm: do not assume that template resolving always returns xfrms")
    Signed-off-by: YueHaibing
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    YueHaibing
     

05 Sep, 2018

2 commits

  • [ Upstream commit 86126b77dcd551ce223e7293bb55854e3df05646 ]

    nlmsg_multicast() always frees the skb, so in case we cannot call
    it we must do that ourselves.

    Fixes: 21ee543edc0dea ("xfrm: fix race between netns cleanup and state expire notification")
    Signed-off-by: Florian Westphal
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Florian Westphal
     
  • [ Upstream commit 8cc88773855f988d6a3bbf102bbd9dd9c828eb81 ]

    Fix missing dst_release() when local broadcast or multicast traffic is
    xfrm policy blocked.

    For IPv4 this results to dst leak: ip_route_output_flow() allocates
    dst_entry via __ip_route_output_key() and passes it to
    xfrm_lookup_route(). xfrm_lookup returns ERR_PTR(-EPERM) that is
    propagated. The dst that was allocated is never released.

    IPv4 local broadcast testcase:
    ping -b 192.168.1.255 &
    sleep 1
    ip xfrm policy add src 0.0.0.0/0 dst 192.168.1.255/32 dir out action block

    IPv4 multicast testcase:
    ping 224.0.0.1 &
    sleep 1
    ip xfrm policy add src 0.0.0.0/0 dst 224.0.0.1/32 dir out action block

    For IPv6 the missing dst_release() causes trouble e.g. when used in netns:
    ip netns add TEST
    ip netns exec TEST ip link set lo up
    ip link add dummy0 type dummy
    ip link set dev dummy0 netns TEST
    ip netns exec TEST ip addr add fd00::1111 dev dummy0
    ip netns exec TEST ip link set dummy0 up
    ip netns exec TEST ping -6 -c 5 ff02::1%dummy0 &
    sleep 1
    ip netns exec TEST ip xfrm policy add src ::/0 dst ff02::1 dir out action block
    wait
    ip netns del TEST

    After netns deletion we see:
    [ 258.239097] unregister_netdevice: waiting for lo to become free. Usage count = 2
    [ 268.279061] unregister_netdevice: waiting for lo to become free. Usage count = 2
    [ 278.367018] unregister_netdevice: waiting for lo to become free. Usage count = 2
    [ 288.375259] unregister_netdevice: waiting for lo to become free. Usage count = 2

    Fixes: ac37e2515c1a ("xfrm: release dst_orig in case of error in xfrm_lookup()")
    Signed-off-by: Tommi Rantala
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Tommi Rantala
     

24 Aug, 2018

1 commit

  • commit 45c180bc29babbedd6b8c01b975780ef44d9d09c upstream.

    struct xfrm_userpolicy_type has two holes, so we should not
    use C99 style initializer.

    KMSAN report:

    BUG: KMSAN: kernel-infoleak in copyout lib/iov_iter.c:140 [inline]
    BUG: KMSAN: kernel-infoleak in _copy_to_iter+0x1b14/0x2800 lib/iov_iter.c:571
    CPU: 1 PID: 4520 Comm: syz-executor841 Not tainted 4.17.0+ #5
    Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
    Call Trace:
    __dump_stack lib/dump_stack.c:77 [inline]
    dump_stack+0x185/0x1d0 lib/dump_stack.c:113
    kmsan_report+0x188/0x2a0 mm/kmsan/kmsan.c:1117
    kmsan_internal_check_memory+0x138/0x1f0 mm/kmsan/kmsan.c:1211
    kmsan_copy_to_user+0x7a/0x160 mm/kmsan/kmsan.c:1253
    copyout lib/iov_iter.c:140 [inline]
    _copy_to_iter+0x1b14/0x2800 lib/iov_iter.c:571
    copy_to_iter include/linux/uio.h:106 [inline]
    skb_copy_datagram_iter+0x422/0xfa0 net/core/datagram.c:431
    skb_copy_datagram_msg include/linux/skbuff.h:3268 [inline]
    netlink_recvmsg+0x6f1/0x1900 net/netlink/af_netlink.c:1959
    sock_recvmsg_nosec net/socket.c:802 [inline]
    sock_recvmsg+0x1d6/0x230 net/socket.c:809
    ___sys_recvmsg+0x3fe/0x810 net/socket.c:2279
    __sys_recvmmsg+0x58e/0xe30 net/socket.c:2391
    do_sys_recvmmsg+0x2a6/0x3e0 net/socket.c:2472
    __do_sys_recvmmsg net/socket.c:2485 [inline]
    __se_sys_recvmmsg net/socket.c:2481 [inline]
    __x64_sys_recvmmsg+0x15d/0x1c0 net/socket.c:2481
    do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:287
    entry_SYSCALL_64_after_hwframe+0x44/0xa9
    RIP: 0033:0x446ce9
    RSP: 002b:00007fc307918db8 EFLAGS: 00000293 ORIG_RAX: 000000000000012b
    RAX: ffffffffffffffda RBX: 00000000006dbc24 RCX: 0000000000446ce9
    RDX: 000000000000000a RSI: 0000000020005040 RDI: 0000000000000003
    RBP: 00000000006dbc20 R08: 0000000020004e40 R09: 0000000000000000
    R10: 0000000040000000 R11: 0000000000000293 R12: 0000000000000000
    R13: 00007ffc8d2df32f R14: 00007fc3079199c0 R15: 0000000000000001

    Uninit was stored to memory at:
    kmsan_save_stack_with_flags mm/kmsan/kmsan.c:279 [inline]
    kmsan_save_stack mm/kmsan/kmsan.c:294 [inline]
    kmsan_internal_chain_origin+0x12b/0x210 mm/kmsan/kmsan.c:685
    kmsan_memcpy_origins+0x11d/0x170 mm/kmsan/kmsan.c:527
    __msan_memcpy+0x109/0x160 mm/kmsan/kmsan_instr.c:413
    __nla_put lib/nlattr.c:569 [inline]
    nla_put+0x276/0x340 lib/nlattr.c:627
    copy_to_user_policy_type net/xfrm/xfrm_user.c:1678 [inline]
    dump_one_policy+0xbe1/0x1090 net/xfrm/xfrm_user.c:1708
    xfrm_policy_walk+0x45a/0xd00 net/xfrm/xfrm_policy.c:1013
    xfrm_dump_policy+0x1c0/0x2a0 net/xfrm/xfrm_user.c:1749
    netlink_dump+0x9b5/0x1550 net/netlink/af_netlink.c:2226
    __netlink_dump_start+0x1131/0x1270 net/netlink/af_netlink.c:2323
    netlink_dump_start include/linux/netlink.h:214 [inline]
    xfrm_user_rcv_msg+0x8a3/0x9b0 net/xfrm/xfrm_user.c:2577
    netlink_rcv_skb+0x37e/0x600 net/netlink/af_netlink.c:2448
    xfrm_netlink_rcv+0xb2/0xf0 net/xfrm/xfrm_user.c:2598
    netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
    netlink_unicast+0x1680/0x1750 net/netlink/af_netlink.c:1336
    netlink_sendmsg+0x104f/0x1350 net/netlink/af_netlink.c:1901
    sock_sendmsg_nosec net/socket.c:629 [inline]
    sock_sendmsg net/socket.c:639 [inline]
    ___sys_sendmsg+0xec8/0x1320 net/socket.c:2117
    __sys_sendmsg net/socket.c:2155 [inline]
    __do_sys_sendmsg net/socket.c:2164 [inline]
    __se_sys_sendmsg net/socket.c:2162 [inline]
    __x64_sys_sendmsg+0x331/0x460 net/socket.c:2162
    do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:287
    entry_SYSCALL_64_after_hwframe+0x44/0xa9
    Local variable description: ----upt.i@dump_one_policy
    Variable was created at:
    dump_one_policy+0x78/0x1090 net/xfrm/xfrm_user.c:1689
    xfrm_policy_walk+0x45a/0xd00 net/xfrm/xfrm_policy.c:1013

    Byte 130 of 137 is uninitialized
    Memory access starts at ffff88019550407f

    Fixes: c0144beaeca42 ("[XFRM] netlink: Use nla_put()/NLA_PUT() variantes")
    Signed-off-by: Eric Dumazet
    Reported-by: syzbot
    Cc: Steffen Klassert
    Cc: Herbert Xu
    Signed-off-by: Steffen Klassert
    Signed-off-by: Greg Kroah-Hartman

    Eric Dumazet
     

30 May, 2018

4 commits

  • [ Upstream commit 9a3fb9fb84cc30577c1b012a6a3efda944684291 ]

    A recent commit introduced a new struct xfrm_trans_cb
    that is used with the sk_buff control buffer. Unfortunately
    it placed the structure in front of the control buffer and
    overlooked that the IPv4/IPv6 control buffer is still needed
    for some layer 4 protocols. As a result the IPv4/IPv6 control
    buffer is overwritten with this structure. Fix this by setting
    a apropriate header in front of the structure.

    Fixes acf568ee859f ("xfrm: Reinject transport-mode packets ...")
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Steffen Klassert
     
  • [ Upstream commit 46c0ef6e1eb95f619d9f62da4332749153db92f7 ]

    In the xfrm_local_error, rcu_read_unlock should be called when afinfo
    is not NULL. because xfrm_state_get_afinfo calls rcu_read_unlock
    if afinfo is NULL.

    Fixes: af5d27c4e12b ("xfrm: remove xfrm_state_put_afinfo")
    Signed-off-by: Taehee Yoo
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Taehee Yoo
     
  • [ Upstream commit b8b549eec8187ac1b12075d69a2d84d89b5e811a ]

    When IPsec offloading was introduced, we accidentally incremented
    the sequence number counter on the xfrm_state by one packet
    too much in the ESN case. This leads to a sequence number gap of
    one packet after each GSO packet. Fix this by setting the sequence
    number to the correct value.

    Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Steffen Klassert
     
  • [ Upstream commit 143a4454daaf0e80a2b9f37159a0d6d2b61e64ed ]

    When xfrm_policy_get_afinfo returns NULL, it will not hold rcu
    read lock. In this case, rcu_read_unlock should not be called
    in xfrm_get_tos, just like other places where it's calling
    xfrm_policy_get_afinfo.

    Fixes: f5e2bb4f5b22 ("xfrm: policy: xfrm_get_tos cannot fail")
    Signed-off-by: Xin Long
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Xin Long
     

19 May, 2018

2 commits

  • commit 75bf50f4aaa1c78d769d854ab3d975884909e4fb upstream.

    copy geniv when cloning the xfrm state.

    x->geniv was not copied to the new state and migration would fail.

    xfrm_do_migrate
    ..
    xfrm_state_clone()
    ..
    ..
    esp_init_aead()
    crypto_alloc_aead()
    crypto_alloc_tfm()
    crypto_find_alg() return EAGAIN and failed

    Signed-off-by: Antony Antony
    Signed-off-by: Steffen Klassert
    Cc: Ben Hutchings
    Signed-off-by: Greg Kroah-Hartman

    Antony Antony
     
  • commit d16b46e4fd8bc6063624605f25b8c0835bb1fbe3 upstream.

    We do not need locking in xfrm_trans_queue because it is designed
    to use per-CPU buffers. However, the original code incorrectly
    used skb_queue_tail which takes the lock. This patch switches
    it to __skb_queue_tail instead.

    Reported-and-tested-by: Artem Savkov
    Fixes: acf568ee859f ("xfrm: Reinject transport-mode packets...")
    Signed-off-by: Herbert Xu
    Signed-off-by: Steffen Klassert
    Signed-off-by: Alistair Strachan
    Signed-off-by: Greg Kroah-Hartman

    Herbert Xu
     

08 Apr, 2018

3 commits

  • commit 19d7df69fdb2636856dc8919de72fc1bf8f79598 upstream.

    We don't have a compat layer for xfrm, so userspace and kernel
    structures have different sizes in this case. This results in
    a broken configuration, so refuse to configure socket policies
    when trying to insert from 32 bit userspace as we do it already
    with policies inserted via netlink.

    Reported-and-tested-by: syzbot+e1a1577ca8bcb47b769a@syzkaller.appspotmail.com
    Signed-off-by: Steffen Klassert
    Signed-off-by: Greg Kroah-Hartman

    Steffen Klassert
     
  • commit 0dcd7876029b58770f769cbb7b484e88e4a305e5 upstream.

    f7c83bcbfaf5 ("net: xfrm: use __this_cpu_read per-cpu helper") added a
    __this_cpu_read() call inside ipcomp_alloc_tfms().

    At the time, __this_cpu_read() required the caller to either not care
    about races or to handle preemption/interrupt issues. 3.15 tightened
    the rules around some per-cpu operations, and now __this_cpu_read()
    should never be used in a preemptible context. On 3.15 and later, we
    need to use this_cpu_read() instead.

    syzkaller reported this leading to the following kernel BUG while
    fuzzing sendmsg:

    BUG: using __this_cpu_read() in preemptible [00000000] code: repro/3101
    caller is ipcomp_init_state+0x185/0x990
    CPU: 3 PID: 3101 Comm: repro Not tainted 4.16.0-rc4-00123-g86f84779d8e9 #154
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
    Call Trace:
    dump_stack+0xb9/0x115
    check_preemption_disabled+0x1cb/0x1f0
    ipcomp_init_state+0x185/0x990
    ? __xfrm_init_state+0x876/0xc20
    ? lock_downgrade+0x5e0/0x5e0
    ipcomp4_init_state+0xaa/0x7c0
    __xfrm_init_state+0x3eb/0xc20
    xfrm_init_state+0x19/0x60
    pfkey_add+0x20df/0x36f0
    ? pfkey_broadcast+0x3dd/0x600
    ? pfkey_sock_destruct+0x340/0x340
    ? pfkey_seq_stop+0x80/0x80
    ? __skb_clone+0x236/0x750
    ? kmem_cache_alloc+0x1f6/0x260
    ? pfkey_sock_destruct+0x340/0x340
    ? pfkey_process+0x62a/0x6f0
    pfkey_process+0x62a/0x6f0
    ? pfkey_send_new_mapping+0x11c0/0x11c0
    ? mutex_lock_io_nested+0x1390/0x1390
    pfkey_sendmsg+0x383/0x750
    ? dump_sp+0x430/0x430
    sock_sendmsg+0xc0/0x100
    ___sys_sendmsg+0x6c8/0x8b0
    ? copy_msghdr_from_user+0x3b0/0x3b0
    ? pagevec_lru_move_fn+0x144/0x1f0
    ? find_held_lock+0x32/0x1c0
    ? do_huge_pmd_anonymous_page+0xc43/0x11e0
    ? lock_downgrade+0x5e0/0x5e0
    ? get_kernel_page+0xb0/0xb0
    ? _raw_spin_unlock+0x29/0x40
    ? do_huge_pmd_anonymous_page+0x400/0x11e0
    ? __handle_mm_fault+0x553/0x2460
    ? __fget_light+0x163/0x1f0
    ? __sys_sendmsg+0xc7/0x170
    __sys_sendmsg+0xc7/0x170
    ? SyS_shutdown+0x1a0/0x1a0
    ? __do_page_fault+0x5a0/0xca0
    ? lock_downgrade+0x5e0/0x5e0
    SyS_sendmsg+0x27/0x40
    ? __sys_sendmsg+0x170/0x170
    do_syscall_64+0x19f/0x640
    entry_SYSCALL_64_after_hwframe+0x42/0xb7
    RIP: 0033:0x7f0ee73dfb79
    RSP: 002b:00007ffe14fc15a8 EFLAGS: 00000207 ORIG_RAX: 000000000000002e
    RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f0ee73dfb79
    RDX: 0000000000000000 RSI: 00000000208befc8 RDI: 0000000000000004
    RBP: 00007ffe14fc15b0 R08: 00007ffe14fc15c0 R09: 00007ffe14fc15c0
    R10: 0000000000000000 R11: 0000000000000207 R12: 0000000000400440
    R13: 00007ffe14fc16b0 R14: 0000000000000000 R15: 0000000000000000

    Signed-off-by: Greg Hackmann
    Signed-off-by: Steffen Klassert
    Signed-off-by: Greg Kroah-Hartman

    Greg Hackmann
     
  • commit d97ca5d714a5334aecadadf696875da40f1fbf3e upstream.

    The sanity test added in ecd7918745234 can be bypassed, validation
    only occurs if XFRM_STATE_ESN flag is set, but rest of code doesn't care
    and just checks if the attribute itself is present.

    So always validate. Alternative is to reject if we have the attribute
    without the flag but that would change abi.

    Reported-by: syzbot+0ab777c27d2bb7588f73@syzkaller.appspotmail.com
    Cc: Mathias Krause
    Fixes: ecd7918745234 ("xfrm_user: ensure user supplied esn replay window is valid")
    Fixes: d8647b79c3b7e ("xfrm: Add user interface for esn and big anti-replay windows")
    Signed-off-by: Florian Westphal
    Signed-off-by: Steffen Klassert
    Signed-off-by: Greg Kroah-Hartman

    Florian Westphal
     

19 Mar, 2018

2 commits

  • [ Upstream commit 0ba23a211360af7b6658e4fcfc571970bbbacc55 ]

    In case of wrap around, replay_esn->oseq_hi is not updated
    before it is tested for it's actual value, leading function
    to fail with overflow indication and packets being dropped.

    This patch updates replay_esn->oseq_hi in the right place.

    Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
    Signed-off-by: Yossef Efraim
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Yossef Efraim
     
  • [ Upstream commit be8f8284cd897af2482d4e54fbc2bdfc15557259 ]

    Currently it is possible to add or update socket policies, but
    not clear them. Therefore, once a socket policy has been applied,
    the socket cannot be used for unencrypted traffic.

    This patch allows (privileged) users to clear socket policies by
    passing in a NULL pointer and zero length argument to the
    {IP,IPV6}_{IPSEC,XFRM}_POLICY setsockopts. This results in both
    the incoming and outgoing policies being cleared.

    The simple approach taken in this patch cannot clear socket
    policies in only one direction. If desired this could be added
    in the future, for example by continuing to pass in a length of
    zero (which currently is guaranteed to return EMSGSIZE) and
    making the policy be a pointer to an integer that contains one
    of the XFRM_POLICY_{IN,OUT} enum values.

    An alternative would have been to interpret the length as a
    signed integer and use XFRM_POLICY_IN (i.e., 0) to clear the
    input policy and -XFRM_POLICY_OUT (i.e., -1) to clear the output
    policy.

    Tested: https://android-review.googlesource.com/539816
    Signed-off-by: Lorenzo Colitti
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Lorenzo Colitti
     

03 Mar, 2018

1 commit

  • [ Upstream commit acf568ee859f098279eadf551612f103afdacb4e ]

    This is an old bugbear of mine:

    https://www.mail-archive.com/netdev@vger.kernel.org/msg03894.html

    By crafting special packets, it is possible to cause recursion
    in our kernel when processing transport-mode packets at levels
    that are only limited by packet size.

    The easiest one is with DNAT, but an even worse one is where
    UDP encapsulation is used in which case you just have to insert
    an UDP encapsulation header in between each level of recursion.

    This patch avoids this problem by reinjecting tranport-mode packets
    through a tasklet.

    Fixes: b05e106698d9 ("[IPV4/6]: Netfilter IPsec input hooks")
    Signed-off-by: Herbert Xu
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Herbert Xu
     

25 Feb, 2018

7 commits

  • [ Upstream commit 732706afe1cc46ef48493b3d2b69c98f36314ae4 ]

    On policies with a transport mode template, we pass the addresses
    from the flowi to xfrm_state_find(), assuming that the IP addresses
    (and address family) don't change during transformation.

    Unfortunately our policy template validation is not strict enough.
    It is possible to configure policies with transport mode template
    where the address family of the template does not match the selectors
    address family. This lead to stack-out-of-bound reads because
    we compare arddesses of the wrong family. Fix this by refusing
    such a configuration, address family can not change on transport
    mode.

    We use the assumption that, on transport mode, the first templates
    address family must match the address family of the policy selector.
    Subsequent transport mode templates must mach the address family of
    the previous template.

    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Steffen Klassert
     
  • [ Upstream commit 4ce3dbe397d7b6b15f272ae757c78c35e9e4b61d ]

    Code path when (encap_type < 0) does not verify the state is valid
    before progressing.

    This will result in a crash if, for instance, x->km.state ==
    XFRM_STATE_ACQ.

    Fixes: 7785bba299a8 ("esp: Add a software GRO codepath")
    Signed-off-by: Aviv Heller
    Signed-off-by: Yevgeny Kliteynik
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Aviv Heller
     
  • commit 6a53b7593233ab9e4f96873ebacc0f653a55c3e1 upstream.

    syzbot reported a kernel warning in xfrm_state_fini(), which
    indicates that we have entries left in the list
    net->xfrm.state_all whose proto is zero. And
    xfrm_id_proto_match() doesn't consider them as a match with
    IPSEC_PROTO_ANY in this case.

    Proto with value 0 is probably not a valid value, at least
    verify_newsa_info() doesn't consider it valid either.

    This patch fixes it by checking the proto value in
    validate_tmpl() and rejecting invalid ones, like what iproute2
    does in xfrm_xfrmproto_getbyname().

    Reported-by: syzbot
    Cc: Steffen Klassert
    Cc: Herbert Xu
    Signed-off-by: Cong Wang
    Signed-off-by: Steffen Klassert
    Signed-off-by: Greg Kroah-Hartman

    Cong Wang
     
  • commit ddc47e4404b58f03e98345398fb12d38fe291512 upstream.

    When we do tunnel or beet mode, we pass saddr and daddr from the
    template to xfrm_state_find(), this is ok. On transport mode,
    we pass the addresses from the flowi, assuming that the IP
    addresses (and address family) don't change during transformation.
    This assumption is wrong in the IPv4 mapped IPv6 case, packet
    is IPv4 and template is IPv6.

    Fix this by catching address family missmatches of the policy
    and the flow already before we do the lookup.

    Reported-by: syzbot
    Signed-off-by: Steffen Klassert
    Signed-off-by: Greg Kroah-Hartman

    Steffen Klassert
     
  • commit 862591bf4f519d1b8d859af720fafeaebdd0162a upstream.

    syzkaller triggered following KASAN splat:

    BUG: KASAN: slab-out-of-bounds in xfrm_hash_rebuild+0xdbe/0xf00 net/xfrm/xfrm_policy.c:618
    read of size 2 at addr ffff8801c8e92fe4 by task kworker/1:1/23 [..]
    Workqueue: events xfrm_hash_rebuild [..]
    __asan_report_load2_noabort+0x14/0x20 mm/kasan/report.c:428
    xfrm_hash_rebuild+0xdbe/0xf00 net/xfrm/xfrm_policy.c:618
    process_one_work+0xbbf/0x1b10 kernel/workqueue.c:2112
    worker_thread+0x223/0x1990 kernel/workqueue.c:2246 [..]

    The reproducer triggers:
    1016 if (error) {
    1017 list_move_tail(&walk->walk.all, &x->all);
    1018 goto out;
    1019 }

    in xfrm_policy_walk() via pfkey (it sets tiny rcv space, dump
    callback returns -ENOBUFS).

    In this case, *walk is located the pfkey socket struct, so this socket
    becomes visible in the global policy list.

    It looks like this is intentional -- phony walker has walk.dead set to 1
    and all other places skip such "policies".

    Ccing original authors of the two commits that seem to expose this
    issue (first patch missed ->dead check, second patch adds pfkey
    sockets to policies dumper list).

    Fixes: 880a6fab8f6ba5b ("xfrm: configure policy hash table thresholds by netlink")
    Fixes: 12a169e7d8f4b1c ("ipsec: Put dumpers on the dump list")
    Cc: Herbert Xu
    Cc: Timo Teras
    Cc: Christophe Gouault
    Reported-by: syzbot
    Signed-off-by: Florian Westphal
    Signed-off-by: Steffen Klassert
    Signed-off-by: Greg Kroah-Hartman

    Florian Westphal
     
  • commit 2f10a61cee8fdb9f8da90f5db687e1862b22cf06 upstream.

    request_module can sleep, thus we cannot hold rcu_read_lock() while
    calling it. The function also jumps back and takes rcu_read_lock()
    again (in xfrm_state_get_afinfo()), resulting in an imbalance.

    This codepath is triggered whenever a new offloaded state is created.

    Fixes: ffdb5211da1c ("xfrm: Auto-load xfrm offload modules")
    Reported-by: syzbot+ca425f44816d749e8eb49755567a75ee48cf4a30@syzkaller.appspotmail.com
    Signed-off-by: Sabrina Dubroca
    Signed-off-by: Steffen Klassert
    Signed-off-by: Greg Kroah-Hartman

    Sabrina Dubroca
     
  • commit b1bdcb59b64f806ef08d25a85c39ffb3ad841ce6 upstream.

    xfrm_policy_cache_flush can sleep, so it cannot be called while holding
    a spinlock. We could release the lock first, but I don't see why we need
    to invoke this function here in first place, the packet path won't reuse
    an xdst entry unless its still valid.

    While at it, add an annotation to xfrm_policy_cache_flush, it would
    have probably caught this bug sooner.

    Fixes: ec30d78c14a813 ("xfrm: add xdst pcpu cache")
    Reported-by: syzbot+e149f7d1328c26f9c12f@syzkaller.appspotmail.com
    Signed-off-by: Florian Westphal
    Signed-off-by: Steffen Klassert
    Signed-off-by: Greg Kroah-Hartman

    Florian Westphal
     

31 Jan, 2018

1 commit

  • commit 76a4201191814a0061cb5c861fafb9ecaa764846 upstream.

    We need to run xfrm_resolve_and_create_bundle() with
    bottom halves off. Otherwise we may reuse an already
    released dst_enty when the xfrm lookup functions are
    called from process context.

    Fixes: c30d78c14a813db39a647b6a348b428 ("xfrm: add xdst pcpu cache")
    Reported-by: Darius Ski
    Signed-off-by: Steffen Klassert
    Acked-by: David Miller ,
    Signed-off-by: Greg Kroah-Hartman

    Steffen Klassert
     

05 Jan, 2018

1 commit


14 Dec, 2017

1 commit

  • [ Upstream commit 0e74aa1d79a5bbc663e03a2804399cae418a0321 ]

    The syzbot found an ancient bug in the IPsec code. When we cloned
    a socket policy (for example, for a child TCP socket derived from a
    listening socket), we did not copy the family field. This results
    in a live policy with a zero family field. This triggers a BUG_ON
    check in the af_key code when the cloned policy is retrieved.

    This patch fixes it by copying the family field over.

    Reported-by: syzbot
    Signed-off-by: Herbert Xu
    Signed-off-by: Steffen Klassert
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Herbert Xu
     

09 Nov, 2017

1 commit

  • Steffen Klassert says:

    ====================
    pull request (net): ipsec 2017-11-09

    1) Fix a use after free due to a reallocated skb head.
    From Florian Westphal.

    2) Fix sporadic lookup failures on labeled IPSEC.
    From Florian Westphal.

    3) Fix a stack out of bounds when a socket policy is applied
    to an IPv6 socket that sends IPv4 packets.

    Please pull or let me know if there are problems.
    ====================

    Signed-off-by: David S. Miller

    David S. Miller
     

03 Nov, 2017

3 commits

  • When we do tunnel or beet mode, we pass saddr and daddr from the
    template to xfrm_state_find(), this is ok. On transport mode,
    we pass the addresses from the flowi, assuming that the IP
    addresses (and address family) don't change during transformation.
    This assumption is wrong in the IPv4 mapped IPv6 case, packet
    is IPv4 and template is IPv6. Fix this by using the addresses
    from the template unconditionally.

    Signed-off-by: Steffen Klassert

    Steffen Klassert
     
  • Stephen Smalley says:
    Since 4.14-rc1, the selinux-testsuite has been encountering sporadic
    failures during testing of labeled IPSEC. git bisect pointed to
    commit ec30d ("xfrm: add xdst pcpu cache").
    The xdst pcpu cache is only checking that the policies are the same,
    but does not validate that the policy, state, and flow match with respect
    to security context labeling.
    As a result, the wrong SA could be used and the receiver could end up
    performing permission checking and providing SO_PEERSEC or SCM_SECURITY
    values for the wrong security context.

    This fix makes it so that we always do the template resolution, and
    then checks that the found states match those in the pcpu bundle.

    This has the disadvantage of doing a bit more work (lookup in state hash
    table) if we can reuse the xdst entry (we only avoid xdst alloc/free)
    but we don't add a lot of extra work in case we can't reuse.

    xfrm_pol_dead() check is removed, reasoning is that
    xfrm_tmpl_resolve does all needed checks.

    Cc: Paul Moore
    Fixes: ec30d78c14a813db39a647b6a348b428 ("xfrm: add xdst pcpu cache")
    Reported-by: Stephen Smalley
    Tested-by: Stephen Smalley
    Signed-off-by: Florian Westphal
    Acked-by: Paul Moore
    Signed-off-by: Steffen Klassert

    Florian Westphal
     
  • …el/git/gregkh/driver-core

    Pull initial SPDX identifiers from Greg KH:
    "License cleanup: add SPDX license identifiers to some files

    Many source files in the tree are missing licensing information, which
    makes it harder for compliance tools to determine the correct license.

    By default all files without license information are under the default
    license of the kernel, which is GPL version 2.

    Update the files which contain no license information with the
    'GPL-2.0' SPDX license identifier. The SPDX identifier is a legally
    binding shorthand, which can be used instead of the full boiler plate
    text.

    This patch is based on work done by Thomas Gleixner and Kate Stewart
    and Philippe Ombredanne.

    How this work was done:

    Patches were generated and checked against linux-4.14-rc6 for a subset
    of the use cases:

    - file had no licensing information it it.

    - file was a */uapi/* one with no licensing information in it,

    - file was a */uapi/* one with existing licensing information,

    Further patches will be generated in subsequent months to fix up cases
    where non-standard license headers were used, and references to
    license had to be inferred by heuristics based on keywords.

    The analysis to determine which SPDX License Identifier to be applied
    to a file was done in a spreadsheet of side by side results from of
    the output of two independent scanners (ScanCode & Windriver)
    producing SPDX tag:value files created by Philippe Ombredanne.
    Philippe prepared the base worksheet, and did an initial spot review
    of a few 1000 files.

    The 4.13 kernel was the starting point of the analysis with 60,537
    files assessed. Kate Stewart did a file by file comparison of the
    scanner results in the spreadsheet to determine which SPDX license
    identifier(s) to be applied to the file. She confirmed any
    determination that was not immediately clear with lawyers working with
    the Linux Foundation.

    Criteria used to select files for SPDX license identifier tagging was:

    - Files considered eligible had to be source code files.

    - Make and config files were included as candidates if they contained
    >5 lines of source

    - File already had some variant of a license header in it (even if <5
    lines).

    All documentation files were explicitly excluded.

    The following heuristics were used to determine which SPDX license
    identifiers to apply.

    - when both scanners couldn't find any license traces, file was
    considered to have no license information in it, and the top level
    COPYING file license applied.

    For non */uapi/* files that summary was:

    SPDX license identifier # files
    ---------------------------------------------------|-------
    GPL-2.0 11139

    and resulted in the first patch in this series.

    If that file was a */uapi/* path one, it was "GPL-2.0 WITH
    Linux-syscall-note" otherwise it was "GPL-2.0". Results of that
    was:

    SPDX license identifier # files
    ---------------------------------------------------|-------
    GPL-2.0 WITH Linux-syscall-note 930

    and resulted in the second patch in this series.

    - if a file had some form of licensing information in it, and was one
    of the */uapi/* ones, it was denoted with the Linux-syscall-note if
    any GPL family license was found in the file or had no licensing in
    it (per prior point). Results summary:

    SPDX license identifier # files
    ---------------------------------------------------|------
    GPL-2.0 WITH Linux-syscall-note 270
    GPL-2.0+ WITH Linux-syscall-note 169
    ((GPL-2.0 WITH Linux-syscall-note) OR BSD-2-Clause) 21
    ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) 17
    LGPL-2.1+ WITH Linux-syscall-note 15
    GPL-1.0+ WITH Linux-syscall-note 14
    ((GPL-2.0+ WITH Linux-syscall-note) OR BSD-3-Clause) 5
    LGPL-2.0+ WITH Linux-syscall-note 4
    LGPL-2.1 WITH Linux-syscall-note 3
    ((GPL-2.0 WITH Linux-syscall-note) OR MIT) 3
    ((GPL-2.0 WITH Linux-syscall-note) AND MIT) 1

    and that resulted in the third patch in this series.

    - when the two scanners agreed on the detected license(s), that
    became the concluded license(s).

    - when there was disagreement between the two scanners (one detected
    a license but the other didn't, or they both detected different
    licenses) a manual inspection of the file occurred.

    - In most cases a manual inspection of the information in the file
    resulted in a clear resolution of the license that should apply
    (and which scanner probably needed to revisit its heuristics).

    - When it was not immediately clear, the license identifier was
    confirmed with lawyers working with the Linux Foundation.

    - If there was any question as to the appropriate license identifier,
    the file was flagged for further research and to be revisited later
    in time.

    In total, over 70 hours of logged manual review was done on the
    spreadsheet to determine the SPDX license identifiers to apply to the
    source files by Kate, Philippe, Thomas and, in some cases,
    confirmation by lawyers working with the Linux Foundation.

    Kate also obtained a third independent scan of the 4.13 code base from
    FOSSology, and compared selected files where the other two scanners
    disagreed against that SPDX file, to see if there was new insights.
    The Windriver scanner is based on an older version of FOSSology in
    part, so they are related.

    Thomas did random spot checks in about 500 files from the spreadsheets
    for the uapi headers and agreed with SPDX license identifier in the
    files he inspected. For the non-uapi files Thomas did random spot
    checks in about 15000 files.

    In initial set of patches against 4.14-rc6, 3 files were found to have
    copy/paste license identifier errors, and have been fixed to reflect
    the correct identifier.

    Additionally Philippe spent 10 hours this week doing a detailed manual
    inspection and review of the 12,461 patched files from the initial
    patch version early this week with:

    - a full scancode scan run, collecting the matched texts, detected
    license ids and scores

    - reviewing anything where there was a license detected (about 500+
    files) to ensure that the applied SPDX license was correct

    - reviewing anything where there was no detection but the patch
    license was not GPL-2.0 WITH Linux-syscall-note to ensure that the
    applied SPDX license was correct

    This produced a worksheet with 20 files needing minor correction. This
    worksheet was then exported into 3 different .csv files for the
    different types of files to be modified.

    These .csv files were then reviewed by Greg. Thomas wrote a script to
    parse the csv files and add the proper SPDX tag to the file, in the
    format that the file expected. This script was further refined by Greg
    based on the output to detect more types of files automatically and to
    distinguish between header and source .c files (which need different
    comment types.) Finally Greg ran the script using the .csv files to
    generate the patches.

    Reviewed-by: Kate Stewart <kstewart@linuxfoundation.org>
    Reviewed-by: Philippe Ombredanne <pombredanne@nexb.com>
    Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>"

    * tag 'spdx_identifiers-4.14-rc8' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core:
    License cleanup: add SPDX license identifier to uapi header files with a license
    License cleanup: add SPDX license identifier to uapi header files with no license
    License cleanup: add SPDX GPL-2.0 license identifier to files with no license

    Linus Torvalds
     

02 Nov, 2017

2 commits

  • syzbot reports:
    BUG: KASAN: use-after-free in __xfrm_state_lookup+0x695/0x6b0
    Read of size 4 at addr ffff8801d434e538 by task syzkaller647520/2991
    [..]
    __xfrm_state_lookup+0x695/0x6b0 net/xfrm/xfrm_state.c:833
    xfrm_state_lookup+0x8a/0x160 net/xfrm/xfrm_state.c:1592
    xfrm_input+0x8e5/0x22f0 net/xfrm/xfrm_input.c:302

    The use-after-free is the ipv4 destination address, which points
    to an skb head area that has been reallocated:
    pskb_expand_head+0x36b/0x1210 net/core/skbuff.c:1494
    __pskb_pull_tail+0x14a/0x17c0 net/core/skbuff.c:1877
    pskb_may_pull include/linux/skbuff.h:2102 [inline]
    xfrm_parse_spi+0x3d3/0x4d0 net/xfrm/xfrm_input.c:170
    xfrm_input+0xce2/0x22f0 net/xfrm/xfrm_input.c:291

    so the real bug is that xfrm_parse_spi() uses pskb_may_pull, but
    for now do smaller workaround that makes xfrm_input fetch daddr
    after spi parsing.

    Reported-by: syzbot
    Signed-off-by: Florian Westphal
    Signed-off-by: Steffen Klassert

    Florian Westphal
     
  • Many source files in the tree are missing licensing information, which
    makes it harder for compliance tools to determine the correct license.

    By default all files without license information are under the default
    license of the kernel, which is GPL version 2.

    Update the files which contain no license information with the 'GPL-2.0'
    SPDX license identifier. The SPDX identifier is a legally binding
    shorthand, which can be used instead of the full boiler plate text.

    This patch is based on work done by Thomas Gleixner and Kate Stewart and
    Philippe Ombredanne.

    How this work was done:

    Patches were generated and checked against linux-4.14-rc6 for a subset of
    the use cases:
    - file had no licensing information it it.
    - file was a */uapi/* one with no licensing information in it,
    - file was a */uapi/* one with existing licensing information,

    Further patches will be generated in subsequent months to fix up cases
    where non-standard license headers were used, and references to license
    had to be inferred by heuristics based on keywords.

    The analysis to determine which SPDX License Identifier to be applied to
    a file was done in a spreadsheet of side by side results from of the
    output of two independent scanners (ScanCode & Windriver) producing SPDX
    tag:value files created by Philippe Ombredanne. Philippe prepared the
    base worksheet, and did an initial spot review of a few 1000 files.

    The 4.13 kernel was the starting point of the analysis with 60,537 files
    assessed. Kate Stewart did a file by file comparison of the scanner
    results in the spreadsheet to determine which SPDX license identifier(s)
    to be applied to the file. She confirmed any determination that was not
    immediately clear with lawyers working with the Linux Foundation.

    Criteria used to select files for SPDX license identifier tagging was:
    - Files considered eligible had to be source code files.
    - Make and config files were included as candidates if they contained >5
    lines of source
    - File already had some variant of a license header in it (even if
    Reviewed-by: Philippe Ombredanne
    Reviewed-by: Thomas Gleixner
    Signed-off-by: Greg Kroah-Hartman

    Greg Kroah-Hartman
     

31 Oct, 2017

1 commit

  • We reset the encapsulation field of the skb too early
    in xfrm_output. As a result, the GRE GSO handler does
    not segment the packets. This leads to a performance
    drop down. We fix this by resetting the encapsulation
    field right before we do the transformation, when
    the inner headers become invalid.

    Fixes: f1bd7d659ef0 ("xfrm: Add encapsulation header offsets while SKB is not encrypted")
    Reported-by: Vicente De Luca
    Signed-off-by: Steffen Klassert

    Steffen Klassert
     

26 Oct, 2017

1 commit

  • If a socket has a valid dst cache, then xfrm_lookup_route will get
    skipped. However, the cache is not invalidated when applying policy to a
    socket (i.e. IPV6_XFRM_POLICY). The result is that new policies are
    sometimes ignored on those sockets. (Note: This was broken for IPv4 and
    IPv6 at different times.)

    This can be demonstrated like so,
    1. Create UDP socket.
    2. connect() the socket.
    3. Apply an outbound XFRM policy to the socket. (setsockopt)
    4. send() data on the socket.

    Packets will continue to be sent in the clear instead of matching an
    xfrm or returning a no-match error (EAGAIN). This affects calls to
    send() and not sendto().

    Invalidating the sk_dst_cache is necessary to correctly apply xfrm
    policies. Since we do this in xfrm_user_policy(), the sk_lock was
    already acquired in either do_ip_setsockopt() or do_ipv6_setsockopt(),
    and we may call __sk_dst_reset().

    Performance impact should be negligible, since this code is only called
    when changing xfrm policy, and only affects the socket in question.

    Fixes: 00bc0ef5880d ("ipv6: Skip XFRM lookup if dst_entry in socket cache is valid")
    Tested: https://android-review.googlesource.com/517555
    Tested: https://android-review.googlesource.com/418659
    Signed-off-by: Jonathan Basseri
    Signed-off-by: Steffen Klassert

    Jonathan Basseri
     

24 Oct, 2017

1 commit

  • We have a memleak whenever a flow matches a policy without
    a matching SA. In this case we generate a dummy bundle and
    take an additional refcount on the dst_entry. This was needed
    as long as we had the flowcache. The flowcache removal patches
    deleted all related refcounts but forgot the one for the
    dummy bundle case. Fix the memleak by removing this refcount.

    Fixes: 3ca28286ea80 ("xfrm_policy: bypass flow_cache_lookup")
    Reported-by: Maxime Bizon
    Signed-off-by: Steffen Klassert

    Steffen Klassert
     

23 Oct, 2017

1 commit

  • An independent security researcher, Mohamed Ghannam, has reported
    this vulnerability to Beyond Security's SecuriTeam Secure Disclosure
    program.

    The xfrm_dump_policy_done function expects xfrm_dump_policy to
    have been called at least once or it will crash. This can be
    triggered if a dump fails because the target socket's receive
    buffer is full.

    This patch fixes it by using the cb->start mechanism to ensure that
    the initialisation is always done regardless of the buffer situation.

    Fixes: 12a169e7d8f4 ("ipsec: Put dumpers on the dump list")
    Signed-off-by: Herbert Xu
    Signed-off-by: Steffen Klassert

    Herbert Xu
     

11 Oct, 2017

1 commit


28 Sep, 2017

1 commit

  • I might be wrong but it doesn't look like xfrm_state_lock is required
    for xfrm_policy_cache_flush and calling it under this lock triggers both
    "sleeping function called from invalid context" and "possible circular
    locking dependency detected" warnings on flush.

    Fixes: ec30d78c14a8 xfrm: add xdst pcpu cache
    Signed-off-by: Artem Savkov
    Acked-by: Florian Westphal
    Signed-off-by: Steffen Klassert

    Artem Savkov
     

13 Sep, 2017

1 commit

  • Can be reproduced with LTP tests:
    # icmp-uni-vti.sh -p ah -a sha256 -m tunnel -S fffffffe -k 1 -s 10

    IPv4:
    RIP: 0010:xfrm_input+0x7f9/0x870
    ...
    Call Trace:

    vti_input+0xaa/0x110 [ip_vti]
    ? skb_free_head+0x21/0x40
    vti_rcv+0x33/0x40 [ip_vti]
    xfrm4_ah_rcv+0x33/0x60
    ip_local_deliver_finish+0x94/0x1e0
    ip_local_deliver+0x6f/0xe0
    ? ip_route_input_noref+0x28/0x50
    ...

    # icmp-uni-vti.sh -6 -p ah -a sha256 -m tunnel -S fffffffe -k 1 -s 10
    IPv6:
    RIP: 0010:xfrm_input+0x7f9/0x870
    ...
    Call Trace:

    xfrm6_rcv_tnl+0x3c/0x40
    vti6_rcv+0xd5/0xe0 [ip6_vti]
    xfrm6_ah_rcv+0x33/0x60
    ip6_input_finish+0xee/0x460
    ip6_input+0x3f/0xb0
    ip6_rcv_finish+0x45/0xa0
    ipv6_rcv+0x34b/0x540

    xfrm_input() invokes xfrm_rcv_cb() -> vti_rcv_cb(), the last callback
    might call skb_scrub_packet(), which in turn can reset secpath.

    Fix it by adding a check that skb->sp is not NULL.

    Fixes: 7e9e9202bccc ("xfrm: Clear RX SKB secpath xfrm_offload")
    Signed-off-by: Alexey Kodanev
    Signed-off-by: Steffen Klassert

    Alexey Kodanev
     

11 Sep, 2017

1 commit