Skip to content

Commit

Permalink
ceph: correctly handle releasing an embedded cap flush
Browse files Browse the repository at this point in the history
The ceph_cap_flush structures are usually dynamically allocated, but
the ceph_cap_snap has an embedded one.

When force umounting, the client will try to remove all the session
caps. During this, it will free them, but that should not be done
with the ones embedded in a capsnap.

Fix this by adding a new boolean that indicates that the cap flush is
embedded in a capsnap, and skip freeing it if that's set.

At the same time, switch to using list_del_init() when detaching the
i_list and g_list heads.  It's possible for a forced umount to remove
these objects but then handle_cap_flushsnap_ack() races in and does the
list_del_init() again, corrupting memory.

Cc: [email protected]
URL: https://tracker.ceph.com/issues/52283
Signed-off-by: Xiubo Li <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: Ilya Dryomov <[email protected]>
  • Loading branch information
lxbsz authored and idryomov committed Aug 25, 2021
1 parent e22ce8e commit b2f9fa1
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 12 deletions.
21 changes: 13 additions & 8 deletions fs/ceph/caps.c
Original file line number Diff line number Diff line change
Expand Up @@ -1743,7 +1743,11 @@ int __ceph_mark_dirty_caps(struct ceph_inode_info *ci, int mask,

struct ceph_cap_flush *ceph_alloc_cap_flush(void)
{
return kmem_cache_alloc(ceph_cap_flush_cachep, GFP_KERNEL);
struct ceph_cap_flush *cf;

cf = kmem_cache_alloc(ceph_cap_flush_cachep, GFP_KERNEL);
cf->is_capsnap = false;
return cf;
}

void ceph_free_cap_flush(struct ceph_cap_flush *cf)
Expand Down Expand Up @@ -1778,7 +1782,7 @@ static bool __detach_cap_flush_from_mdsc(struct ceph_mds_client *mdsc,
prev->wake = true;
wake = false;
}
list_del(&cf->g_list);
list_del_init(&cf->g_list);
return wake;
}

Expand All @@ -1793,7 +1797,7 @@ static bool __detach_cap_flush_from_ci(struct ceph_inode_info *ci,
prev->wake = true;
wake = false;
}
list_del(&cf->i_list);
list_del_init(&cf->i_list);
return wake;
}

Expand Down Expand Up @@ -2352,7 +2356,7 @@ static void __kick_flushing_caps(struct ceph_mds_client *mdsc,
ci->i_ceph_flags &= ~CEPH_I_KICK_FLUSH;

list_for_each_entry_reverse(cf, &ci->i_cap_flush_list, i_list) {
if (!cf->caps) {
if (cf->is_capsnap) {
last_snap_flush = cf->tid;
break;
}
Expand All @@ -2371,7 +2375,7 @@ static void __kick_flushing_caps(struct ceph_mds_client *mdsc,

first_tid = cf->tid + 1;

if (cf->caps) {
if (!cf->is_capsnap) {
struct cap_msg_args arg;

dout("kick_flushing_caps %p cap %p tid %llu %s\n",
Expand Down Expand Up @@ -3516,7 +3520,7 @@ static void handle_cap_flush_ack(struct inode *inode, u64 flush_tid,
cleaned = cf->caps;

/* Is this a capsnap? */
if (cf->caps == 0)
if (cf->is_capsnap)
continue;

if (cf->tid <= flush_tid) {
Expand Down Expand Up @@ -3589,8 +3593,9 @@ static void handle_cap_flush_ack(struct inode *inode, u64 flush_tid,
while (!list_empty(&to_remove)) {
cf = list_first_entry(&to_remove,
struct ceph_cap_flush, i_list);
list_del(&cf->i_list);
ceph_free_cap_flush(cf);
list_del_init(&cf->i_list);
if (!cf->is_capsnap)
ceph_free_cap_flush(cf);
}

if (wake_ci)
Expand Down
7 changes: 4 additions & 3 deletions fs/ceph/mds_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -1616,7 +1616,7 @@ static int remove_session_caps_cb(struct inode *inode, struct ceph_cap *cap,
spin_lock(&mdsc->cap_dirty_lock);

list_for_each_entry(cf, &to_remove, i_list)
list_del(&cf->g_list);
list_del_init(&cf->g_list);

if (!list_empty(&ci->i_dirty_item)) {
pr_warn_ratelimited(
Expand Down Expand Up @@ -1668,8 +1668,9 @@ static int remove_session_caps_cb(struct inode *inode, struct ceph_cap *cap,
struct ceph_cap_flush *cf;
cf = list_first_entry(&to_remove,
struct ceph_cap_flush, i_list);
list_del(&cf->i_list);
ceph_free_cap_flush(cf);
list_del_init(&cf->i_list);
if (!cf->is_capsnap)
ceph_free_cap_flush(cf);
}

wake_up_all(&ci->i_cap_wq);
Expand Down
3 changes: 3 additions & 0 deletions fs/ceph/snap.c
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,9 @@ static void ceph_queue_cap_snap(struct ceph_inode_info *ci)
pr_err("ENOMEM allocating ceph_cap_snap on %p\n", inode);
return;
}
capsnap->cap_flush.is_capsnap = true;
INIT_LIST_HEAD(&capsnap->cap_flush.i_list);
INIT_LIST_HEAD(&capsnap->cap_flush.g_list);

spin_lock(&ci->i_ceph_lock);
used = __ceph_caps_used(ci);
Expand Down
3 changes: 2 additions & 1 deletion fs/ceph/super.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,9 @@ struct ceph_cap {

struct ceph_cap_flush {
u64 tid;
int caps; /* 0 means capsnap */
int caps;
bool wake; /* wake up flush waiters when finish ? */
bool is_capsnap; /* true means capsnap */
struct list_head g_list; // global
struct list_head i_list; // per inode
};
Expand Down

0 comments on commit b2f9fa1

Please sign in to comment.