• Ilya Dryomov's avatar
    libceph: fix potential use-after-free on linger ping and resends · 75dbb685
    Ilya Dryomov authored
    request_reinit() is not only ugly as the comment rightfully suggests,
    but also unsafe.  Even though it is called with osdc->lock held for
    write in all cases, resetting the OSD request refcount can still race
    with handle_reply() and result in use-after-free.  Taking linger ping
    as an example:
    
        handle_timeout thread                     handle_reply thread
    
                                                  down_read(&osdc->lock)
                                                  req = lookup_request(...)
                                                  ...
                                                  finish_request(req)  # unregisters
                                                  up_read(&osdc->lock)
                                                  __complete_request(req)
                                                    linger_ping_cb(req)
    
          # req->r_kref == 2 because handle_reply still holds its ref
    
        down_write(&osdc->lock)
        send_linger_ping(lreq)
          req = lreq->ping_req  # same req
          # cancel_linger_request is NOT
          # called - handle_reply already
          # unregistered
          request_reinit(req)
            WARN_ON(req->r_kref != 1)  # fires
            request_init(req)
              kref_init(req->r_kref)
    
                       # req->r_kref == 1 after kref_init
    
                                                  ceph_osdc_put_request(req)
                                                    kref_put(req->r_kref)
    
                # req->r_kref == 0 after kref_put, req is freed
    
            <further req initialization/use> !!!
    
    This happens because send_linger_ping() always (re)uses the same OSD
    request for watch ping requests, relying on cancel_linger_request() to
    unregister it from the OSD client and rip its messages out from the
    messenger.  send_linger() does the same for watch/notify registration
    and watch reconnect requests.  Unfortunately cancel_request() doesn't
    guarantee that after it returns the OSD client would be completely done
    with the OSD request -- a ref could still be held and the callback (if
    specified) could still be invoked too.
    
    The original motivation for request_reinit() was inability to deal with
    allocation failures in send_linger() and send_linger_ping().  Switching
    to using osdc->req_mempool (currently only used by CephFS) respects that
    and allows us to get rid of request_reinit().
    
    Cc: stable@vger.kernel.org
    Signed-off-by: default avatarIlya Dryomov <idryomov@gmail.com>
    Reviewed-by: default avatarXiubo Li <xiubli@redhat.com>
    Acked-by: default avatarJeff Layton <jlayton@kernel.org>
    75dbb685
osd_client.c 146 KB