From 0910bab5e5b957e11f356d525eccccfd36d334f9 Mon Sep 17 00:00:00 2001 From: Pranith Kumar K Date: Wed, 20 May 2015 23:56:17 +0530 Subject: cluster/ec: Fix use after free crash ec_heal creates ec_fop_data but doesn't run ec_manager. ec_fop_data_allocate adds this fop to ec->pending_fops, because ec_manager is not run on this heal fop it is never removed from ec->pending_fops. When it is accessed after free it leads to crash. It is better to not to add HEAL fops to ec->pending_fops because we don't want graph switch to hang the mount because of a BIG file/directory heal. BUG: 1188145 Change-Id: I8abdc92f06e0563192300ca4abca3909efcca9c3 Signed-off-by: Pranith Kumar K Reviewed-on: http://review.gluster.org/10868 Reviewed-by: Xavier Hernandez Tested-by: Gluster Build System Reviewed-by: Raghavendra Bhat --- xlators/cluster/ec/src/ec-common.c | 14 ----------- xlators/cluster/ec/src/ec-common.h | 3 +++ xlators/cluster/ec/src/ec-data.c | 50 +++++++++++++++++++++++++++++++++----- xlators/cluster/ec/src/ec-fops.h | 3 --- 4 files changed, 47 insertions(+), 23 deletions(-) (limited to 'xlators') diff --git a/xlators/cluster/ec/src/ec-common.c b/xlators/cluster/ec/src/ec-common.c index 9f312e0c37c..374739ac6e0 100644 --- a/xlators/cluster/ec/src/ec-common.c +++ b/xlators/cluster/ec/src/ec-common.c @@ -1767,21 +1767,7 @@ void __ec_manager(ec_fop_data_t * fop, int32_t error) } if ((fop->state == EC_STATE_END) || (fop->state == -EC_STATE_END)) { - gf_boolean_t notify; - - LOCK(&ec->lock); - - list_del_init(&fop->pending_list); - notify = list_empty(&ec->pending_fops); - - UNLOCK(&ec->lock); - ec_fop_data_release(fop); - - if (notify) { - ec_pending_fops_completed(ec); - } - break; } diff --git a/xlators/cluster/ec/src/ec-common.h b/xlators/cluster/ec/src/ec-common.h index 08993f03c8f..78cf261feeb 100644 --- a/xlators/cluster/ec/src/ec-common.h +++ b/xlators/cluster/ec/src/ec-common.h @@ -20,6 +20,9 @@ typedef enum { EC_METADATA_TXN } ec_txn_t; +#define EC_FOP_HEAL -1 +#define EC_FOP_FHEAL -2 + #define EC_CONFIG_VERSION 0 #define EC_CONFIG_ALGORITHM 0 diff --git a/xlators/cluster/ec/src/ec-data.c b/xlators/cluster/ec/src/ec-data.c index b747fc42348..609a47b466c 100644 --- a/xlators/cluster/ec/src/ec-data.c +++ b/xlators/cluster/ec/src/ec-data.c @@ -96,6 +96,19 @@ void ec_cbk_data_destroy(ec_cbk_data_t * cbk) mem_put(cbk); } +/* PARENT_DOWN will be notified to children only after these fops are complete + * when graph switch happens. We do not want graph switch to be waiting on + * heal to complete as healing big file/directory could take a while. Which + * will lead to hang on the mount. + */ +static inline gf_boolean_t +ec_needs_graceful_completion (ec_fop_data_t *fop) +{ + if ((fop->id != EC_FOP_HEAL) && (fop->id != EC_FOP_FHEAL)) + return _gf_true; + return _gf_false; +} + ec_fop_data_t * ec_fop_data_allocate(call_frame_t * frame, xlator_t * this, int32_t id, uint32_t flags, uintptr_t target, int32_t minimum, @@ -114,6 +127,10 @@ ec_fop_data_t * ec_fop_data_allocate(call_frame_t * frame, xlator_t * this, return NULL; } + INIT_LIST_HEAD(&fop->cbk_list); + INIT_LIST_HEAD(&fop->answer_list); + INIT_LIST_HEAD(&fop->pending_list); + fop->xl = this; fop->req_frame = frame; @@ -148,9 +165,6 @@ ec_fop_data_t * ec_fop_data_allocate(call_frame_t * frame, xlator_t * this, fop->minimum = minimum; fop->mask = target; - INIT_LIST_HEAD(&fop->cbk_list); - INIT_LIST_HEAD(&fop->answer_list); - fop->wind = wind; fop->handler = handler; fop->cbks = cbks; @@ -171,11 +185,13 @@ ec_fop_data_t * ec_fop_data_allocate(call_frame_t * frame, xlator_t * this, fop->parent = parent; } - LOCK(&ec->lock); + if (ec_needs_graceful_completion (fop)) { + LOCK(&ec->lock); - list_add_tail(&fop->pending_list, &ec->pending_fops); + list_add_tail(&fop->pending_list, &ec->pending_fops); - UNLOCK(&ec->lock); + UNLOCK(&ec->lock); + } return fop; } @@ -191,10 +207,27 @@ void ec_fop_data_acquire(ec_fop_data_t * fop) UNLOCK(&fop->lock); } +static void +ec_handle_last_pending_fop_completion (ec_fop_data_t *fop, gf_boolean_t *notify) +{ + ec_t *ec = fop->xl->private; + + if (!list_empty (&fop->pending_list)) { + LOCK(&ec->lock); + { + list_del_init (&fop->pending_list); + *notify = list_empty (&ec->pending_fops); + } + UNLOCK(&ec->lock); + } +} + void ec_fop_data_release(ec_fop_data_t * fop) { + ec_t *ec = NULL; ec_cbk_data_t * cbk, * tmp; int32_t refs; + gf_boolean_t notify = _gf_false; LOCK(&fop->lock); @@ -246,6 +279,11 @@ void ec_fop_data_release(ec_fop_data_t * fop) ec_cbk_data_destroy(cbk); } + ec = fop->xl->private; + ec_handle_last_pending_fop_completion (fop, ¬ify); mem_put(fop); + if (notify) { + ec_pending_fops_completed(ec); + } } } diff --git a/xlators/cluster/ec/src/ec-fops.h b/xlators/cluster/ec/src/ec-fops.h index d6b9770f720..7661077cca3 100644 --- a/xlators/cluster/ec/src/ec-fops.h +++ b/xlators/cluster/ec/src/ec-fops.h @@ -16,9 +16,6 @@ #include "ec-data.h" #include "ec-common.h" -#define EC_FOP_HEAL -1 -#define EC_FOP_FHEAL -2 - void ec_access(call_frame_t * frame, xlator_t * this, uintptr_t target, int32_t minimum, fop_access_cbk_t func, void *data, loc_t * loc, int32_t mask, dict_t * xdata); -- cgit