rbd: fix safety of rbd_put_client()
authorAlex Elder <elder@dreamhost.com>
Sun, 29 Jan 2012 19:57:43 +0000 (13:57 -0600)
committerSage Weil <sage@newdream.net>
Thu, 2 Feb 2012 20:56:59 +0000 (12:56 -0800)
commitd23a4b3fd6ef70b80411b39b8c8bc548a219ce70
treef42dbc4c8544acce987e35df5b3002f1def54cff
parent97bb59a03dd6767fcc00be09b0c6d9e5294eeea6
rbd: fix safety of rbd_put_client()

The rbd_client structure uses a kref to arrange for cleaning up and
freeing an instance when its last reference is dropped.  The cleanup
routine is rbd_client_release(), and one of the things it does is
delete the rbd_client from rbd_client_list.  It acquires node_lock
to do so, but the way it is done is still not safe.

The problem is that when attempting to reuse an existing rbd_client,
the structure found might already be in the process of getting
destroyed and cleaned up.

Here's the scenario, with "CLIENT" representing an existing
rbd_client that's involved in the race:

 Thread on CPU A                | Thread on CPU B
 ---------------                | ---------------
 rbd_put_client(CLIENT)         | rbd_get_client()
   kref_put()                   |   (acquires node_lock)
     kref->refcount becomes 0   |   __rbd_client_find() returns CLIENT
     calls rbd_client_release() |   kref_get(&CLIENT->kref);
                                |   (releases node_lock)
       (acquires node_lock)     |
       deletes CLIENT from list | ...and starts using CLIENT...
       (releases node_lock)     |
       and frees CLIENT         | <-- but CLIENT gets freed here

Fix this by having rbd_put_client() acquire node_lock.  The result
could still be improved, but at least it avoids this problem.

Signed-off-by: Alex Elder <elder@dreamhost.com>
Signed-off-by: Sage Weil <sage@newdream.net>
drivers/block/rbd.c