diff options
author | Pranith Kumar K <pkarampu@redhat.com> | 2015-05-20 23:56:17 +0530 |
---|---|---|
committer | Pranith Kumar Karampuri <pkarampu@redhat.com> | 2015-05-21 06:08:17 -0700 |
commit | 0910bab5e5b957e11f356d525eccccfd36d334f9 (patch) | |
tree | 6fe365154b34dac20751191720139550d135d280 /xlators/cluster/ec/src/ec-data.c | |
parent | a61c788de737f81b9c21b176a26e21a078176ed9 (diff) |
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 <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/10868
Reviewed-by: Xavier Hernandez <xhernandez@datalab.es>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Raghavendra Bhat <raghavendra@redhat.com>
Diffstat (limited to 'xlators/cluster/ec/src/ec-data.c')
-rw-r--r-- | xlators/cluster/ec/src/ec-data.c | 50 |
1 files changed, 44 insertions, 6 deletions
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); + } } } |