• Xin Long's avatar
    esp: remove the skb from the chain when it's enqueued in cryptd_wq · d1d17a35
    Xin Long authored
    Xiumei found a panic in esp offload:
    
      BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
      RIP: 0010:esp_output_done+0x101/0x160 [esp4]
      Call Trace:
       ? esp_output+0x180/0x180 [esp4]
       cryptd_aead_crypt+0x4c/0x90
       cryptd_queue_worker+0x6e/0xa0
       process_one_work+0x1a7/0x3b0
       worker_thread+0x30/0x390
       ? create_worker+0x1a0/0x1a0
       kthread+0x112/0x130
       ? kthread_flush_work_fn+0x10/0x10
       ret_from_fork+0x35/0x40
    
    It was caused by that skb secpath is used in esp_output_done() after it's
    been released elsewhere.
    
    The tx path for esp offload is:
    
      __dev_queue_xmit()->
        validate_xmit_skb_list()->
          validate_xmit_xfrm()->
            esp_xmit()->
              esp_output_tail()->
                aead_request_set_callback(esp_output_done) <--[1]
                crypto_aead_encrypt()  <--[2]
    
    In [1], .callback is set, and in [2] it will trigger the worker schedule,
    later on a kernel thread will call .callback(esp_output_done), as the call
    trace shows.
    
    But in validate_xmit_xfrm():
    
      skb_list_walk_safe(skb, skb2, nskb) {
        ...
        err = x->type_offload->xmit(x, skb2, esp_features);  [esp_xmit]
        ...
      }
    
    When the err is -EINPROGRESS, which means this skb2 will be enqueued and
    later gets encrypted and sent out by .callback later in a kernel thread,
    skb2 should be removed fromt skb chain. Otherwise, it will get processed
    again outside validate_xmit_xfrm(), which could release skb secpath, and
    cause the panic above.
    
    This patch is to remove the skb from the chain when it's enqueued in
    cryptd_wq. While at it, remove the unnecessary 'if (!skb)' check.
    
    Fixes: 3dca3f38 ("xfrm: Separate ESP handling from segmentation for GRO packets.")
    Reported-by: default avatarXiumei Mu <xmu@redhat.com>
    Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
    Signed-off-by: default avatarSteffen Klassert <steffen.klassert@secunet.com>
    d1d17a35
xfrm_device.c 8.77 KB