• Ilya Dryomov's avatar
    libceph: clear msg->con in ceph_msg_release() only · 583d0fef
    Ilya Dryomov authored
    The following bit in ceph_msg_revoke_incoming() is unsafe:
    
        struct ceph_connection *con = msg->con;
        if (!con)
                return;
        mutex_lock(&con->mutex);
        <more msg->con use>
    
    There is nothing preventing con from getting destroyed right after
    msg->con test.  One easy way to reproduce this is to disable message
    signing only on the server side and try to map an image.  The system
    will go into a
    
        libceph: read_partial_message ffff880073f0ab68 signature check failed
        libceph: osd0 192.168.255.155:6801 bad crc/signature
        libceph: read_partial_message ffff880073f0ab68 signature check failed
        libceph: osd0 192.168.255.155:6801 bad crc/signature
    
    loop which has to be interrupted with Ctrl-C.  Hit Ctrl-C and you are
    likely to end up with a random GP fault if the reset handler executes
    "within" ceph_msg_revoke_incoming():
    
                         <yet another reply w/o a signature>
                                       ...
              <Ctrl-C>
        rbd_obj_request_end
          ceph_osdc_cancel_request
            __unregister_request
              ceph_osdc_put_request
                ceph_msg_revoke_incoming
                                       ...
                                    osd_reset
                                      __kick_osd_requests
                                        __reset_osd
                                          remove_osd
                                            ceph_con_close
                                              reset_connection
                                                <clear con->in_msg->con>
                                                <put con ref>
                                                  put_osd
                                                    <free osd/con>
                  <msg->con use> <-- !!!
    
    If ceph_msg_revoke_incoming() executes "before" the reset handler,
    osd/con will be leaked because ceph_msg_revoke_incoming() clears
    con->in_msg but doesn't put con ref, while reset_connection() only puts
    con ref if con->in_msg != NULL.
    
    The current msg->con scheme was introduced by commits 38941f80
    ("libceph: have messages point to their connection") and 92ce034b
    ("libceph: have messages take a connection reference"), which defined
    when messages get associated with a connection and when that
    association goes away.  Part of the problem is that this association is
    supposed to go away in much too many places; closing this race entirely
    requires either a rework of the existing or an addition of a new layer
    of synchronization.
    
    In lieu of that, we can make it *much* less likely to hit by
    disassociating messages only on their destruction and resend through
    a different connection.  This makes the code simpler and is probably
    a good thing to do regardless - this patch adds a msg_con_set() helper
    which is is called from only three places: ceph_con_send() and
    ceph_con_in_msg_alloc() to set msg->con and ceph_msg_release() to clear
    it.
    Signed-off-by: default avatarIlya Dryomov <idryomov@gmail.com>
    583d0fef
osd_client.c 78.5 KB