Commit f818a736 authored by Sage Weil's avatar Sage Weil

ceph: fix cap removal races

The iterate_session_caps helper traverses the session caps list and tries
to grab an inode reference.  However, the __ceph_remove_cap was clearing
the inode backpointer _before_ removing itself from the session list,
causing a null pointer dereference.

Clear cap->ci under protection of s_cap_lock to avoid the race, and to
tightly couple the list and backpointer state.  Use a local flag to
indicate whether we are releasing the cap, as cap->session may be modified
by a racing thread in iterate_session_caps.
Signed-off-by: default avatarSage Weil <sage@newdream.net>
parent 45c6ceb5
...@@ -858,6 +858,8 @@ static int __ceph_is_any_caps(struct ceph_inode_info *ci) ...@@ -858,6 +858,8 @@ static int __ceph_is_any_caps(struct ceph_inode_info *ci)
} }
/* /*
* Remove a cap. Take steps to deal with a racing iterate_session_caps.
*
* caller should hold i_lock. * caller should hold i_lock.
* caller will not hold session s_mutex if called from destroy_inode. * caller will not hold session s_mutex if called from destroy_inode.
*/ */
...@@ -866,15 +868,10 @@ void __ceph_remove_cap(struct ceph_cap *cap) ...@@ -866,15 +868,10 @@ void __ceph_remove_cap(struct ceph_cap *cap)
struct ceph_mds_session *session = cap->session; struct ceph_mds_session *session = cap->session;
struct ceph_inode_info *ci = cap->ci; struct ceph_inode_info *ci = cap->ci;
struct ceph_mds_client *mdsc = &ceph_client(ci->vfs_inode.i_sb)->mdsc; struct ceph_mds_client *mdsc = &ceph_client(ci->vfs_inode.i_sb)->mdsc;
int removed = 0;
dout("__ceph_remove_cap %p from %p\n", cap, &ci->vfs_inode); dout("__ceph_remove_cap %p from %p\n", cap, &ci->vfs_inode);
/* remove from inode list */
rb_erase(&cap->ci_node, &ci->i_caps);
cap->ci = NULL;
if (ci->i_auth_cap == cap)
ci->i_auth_cap = NULL;
/* remove from session list */ /* remove from session list */
spin_lock(&session->s_cap_lock); spin_lock(&session->s_cap_lock);
if (session->s_cap_iterator == cap) { if (session->s_cap_iterator == cap) {
...@@ -885,10 +882,18 @@ void __ceph_remove_cap(struct ceph_cap *cap) ...@@ -885,10 +882,18 @@ void __ceph_remove_cap(struct ceph_cap *cap)
list_del_init(&cap->session_caps); list_del_init(&cap->session_caps);
session->s_nr_caps--; session->s_nr_caps--;
cap->session = NULL; cap->session = NULL;
removed = 1;
} }
/* protect backpointer with s_cap_lock: see iterate_session_caps */
cap->ci = NULL;
spin_unlock(&session->s_cap_lock); spin_unlock(&session->s_cap_lock);
if (cap->session == NULL) /* remove from inode list */
rb_erase(&cap->ci_node, &ci->i_caps);
if (ci->i_auth_cap == cap)
ci->i_auth_cap = NULL;
if (removed)
ceph_put_cap(cap); ceph_put_cap(cap);
if (!__ceph_is_any_caps(ci) && ci->i_snap_realm) { if (!__ceph_is_any_caps(ci) && ci->i_snap_realm) {
......
...@@ -736,9 +736,10 @@ static void cleanup_cap_releases(struct ceph_mds_session *session) ...@@ -736,9 +736,10 @@ static void cleanup_cap_releases(struct ceph_mds_session *session)
} }
/* /*
* Helper to safely iterate over all caps associated with a session. * Helper to safely iterate over all caps associated with a session, with
* special care taken to handle a racing __ceph_remove_cap().
* *
* caller must hold session s_mutex * Caller must hold session s_mutex.
*/ */
static int iterate_session_caps(struct ceph_mds_session *session, static int iterate_session_caps(struct ceph_mds_session *session,
int (*cb)(struct inode *, struct ceph_cap *, int (*cb)(struct inode *, struct ceph_cap *,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment