• O
    can: can_create_echo_skb(): fix echo skb generation: always use skb_clone() · 286228d3
    Oleksij Rempel 提交于
    All user space generated SKBs are owned by a socket (unless injected into the
    key via AF_PACKET). If a socket is closed, all associated skbs will be cleaned
    up.
    
    This leads to a problem when a CAN driver calls can_put_echo_skb() on a
    unshared SKB. If the socket is closed prior to the TX complete handler,
    can_get_echo_skb() and the subsequent delivering of the echo SKB to all
    registered callbacks, a SKB with a refcount of 0 is delivered.
    
    To avoid the problem, in can_get_echo_skb() the original SKB is now always
    cloned, regardless of shared SKB or not. If the process exists it can now
    safely discard its SKBs, without disturbing the delivery of the echo SKB.
    
    The problem shows up in the j1939 stack, when it clones the incoming skb, which
    detects the already 0 refcount.
    
    We can easily reproduce this with following example:
    
    testj1939 -B -r can0: &
    cansend can0 1823ff40#0123
    
    WARNING: CPU: 0 PID: 293 at lib/refcount.c:25 refcount_warn_saturate+0x108/0x174
    refcount_t: addition on 0; use-after-free.
    Modules linked in: coda_vpu imx_vdoa videobuf2_vmalloc dw_hdmi_ahb_audio vcan
    CPU: 0 PID: 293 Comm: cansend Not tainted 5.5.0-rc6-00376-g9e20dcb7040d #1
    Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
    Backtrace:
    [<c010f570>] (dump_backtrace) from [<c010f90c>] (show_stack+0x20/0x24)
    [<c010f8ec>] (show_stack) from [<c0c3e1a4>] (dump_stack+0x8c/0xa0)
    [<c0c3e118>] (dump_stack) from [<c0127fec>] (__warn+0xe0/0x108)
    [<c0127f0c>] (__warn) from [<c01283c8>] (warn_slowpath_fmt+0xa8/0xcc)
    [<c0128324>] (warn_slowpath_fmt) from [<c0539c0c>] (refcount_warn_saturate+0x108/0x174)
    [<c0539b04>] (refcount_warn_saturate) from [<c0ad2cac>] (j1939_can_recv+0x20c/0x210)
    [<c0ad2aa0>] (j1939_can_recv) from [<c0ac9dc8>] (can_rcv_filter+0xb4/0x268)
    [<c0ac9d14>] (can_rcv_filter) from [<c0aca2cc>] (can_receive+0xb0/0xe4)
    [<c0aca21c>] (can_receive) from [<c0aca348>] (can_rcv+0x48/0x98)
    [<c0aca300>] (can_rcv) from [<c09b1fdc>] (__netif_receive_skb_one_core+0x64/0x88)
    [<c09b1f78>] (__netif_receive_skb_one_core) from [<c09b2070>] (__netif_receive_skb+0x38/0x94)
    [<c09b2038>] (__netif_receive_skb) from [<c09b2130>] (netif_receive_skb_internal+0x64/0xf8)
    [<c09b20cc>] (netif_receive_skb_internal) from [<c09b21f8>] (netif_receive_skb+0x34/0x19c)
    [<c09b21c4>] (netif_receive_skb) from [<c0791278>] (can_rx_offload_napi_poll+0x58/0xb4)
    
    Fixes: 0ae89beb ("can: add destructor for self generated skbs")
    Signed-off-by: NOleksij Rempel <o.rempel@pengutronix.de>
    Link: http://lore.kernel.org/r/20200124132656.22156-1-o.rempel@pengutronix.deAcked-by: NOliver Hartkopp <socketcan@hartkopp.net>
    Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>
    286228d3
skb.h 1.8 KB