From: shadow Date: Fri, 6 Feb 2009 12:31:22 +0000 (+0000) Subject: resolve lots races and use after free. X-Git-Tag: v1_9_160~36 X-Git-Url: https://git.whamcloud.com/gitweb?a=commitdiff_plain;ds=sidebyside;h=da0dca9d54757963322ba3458a21c54800e36571;p=fs%2Flustre-release.git resolve lots races and use after free. Branch b1_8 b=HEAD i=jc.lafoucriere i=panda --- diff --git a/lustre/include/class_hash.h b/lustre/include/class_hash.h index 6210c7f..37bd8d2 100644 --- a/lustre/include/class_hash.h +++ b/lustre/include/class_hash.h @@ -85,11 +85,9 @@ lh_hash(lustre_hash_t *lh, void *key, unsigned mask) { LASSERT(lh); LASSERT(LHO(lh)); + LASSERT(LHP(lh, hash)); - if (LHP(lh, hash)) - return LHP(lh, hash)(lh, key, mask); - - return -EOPNOTSUPP; + return LHP(lh, hash)(lh, key, mask); } static inline void * diff --git a/lustre/lov/lov_internal.h b/lustre/lov/lov_internal.h index b3a763a..2aaaff4 100644 --- a/lustre/lov/lov_internal.h +++ b/lustre/lov/lov_internal.h @@ -320,5 +320,6 @@ int lov_pool_remove(struct obd_device *obd, char *poolname, char *ostname); void lov_dump_pool(int level, struct pool_desc *pool); struct pool_desc *lov_find_pool(struct lov_obd *lov, char *poolname); int lov_check_index_in_pool(__u32 idx, struct pool_desc *pool); +void lov_pool_putref(struct pool_desc *pool); #endif diff --git a/lustre/lov/lov_obd.c b/lustre/lov/lov_obd.c index 11267e7..00ce37d 100644 --- a/lustre/lov/lov_obd.c +++ b/lustre/lov/lov_obd.c @@ -841,11 +841,12 @@ int lov_setup(struct obd_device *obd, struct lustre_cfg *lcfg) static int lov_precleanup(struct obd_device *obd, enum obd_cleanup_stage stage) { int rc = 0; + struct lov_obd *lov = &obd->u.lov; + ENTRY; switch (stage) { case OBD_CLEANUP_EARLY: { - struct lov_obd *lov = &obd->u.lov; int i; for (i = 0; i < lov->desc.ld_tgt_count; i++) { if (!lov->lov_tgts[i] || !lov->lov_tgts[i]->ltd_active) @@ -870,22 +871,19 @@ static int lov_cleanup(struct obd_device *obd) struct list_head *pos, *tmp; struct pool_desc *pool; - lprocfs_obd_cleanup(obd); - - /* Delete hash entries and kill hash table before freeing pools - * and get to use after free issue. */ - lustre_hash_exit(lov->lov_pools_hash_body); - list_for_each_safe(pos, tmp, &lov->lov_pool_list) { pool = list_entry(pos, struct pool_desc, pool_list); /* free pool structs */ + CDEBUG(D_INFO, "delete pool %p\n", pool); lov_pool_del(obd, pool->pool_name); } + lustre_hash_exit(lov->lov_pools_hash_body); lov_ost_pool_free(&(lov->lov_qos.lq_rr.lqr_pool)); lov_ost_pool_free(&lov->lov_packed); if (lov->lov_tgts) { int i; + lov_getref(obd); for (i = 0; i < lov->desc.ld_tgt_count; i++) { if (!lov->lov_tgts[i]) continue; @@ -902,11 +900,15 @@ static int lov_cleanup(struct obd_device *obd) atomic_read(&lov->lov_refcount)); lov_del_target(obd, i, 0, 0); } + lov_putref(obd); OBD_FREE(lov->lov_tgts, sizeof(*lov->lov_tgts) * lov->lov_tgt_size); lov->lov_tgt_size = 0; } + /* clear pools parent proc entry only after all pools is killed */ + lprocfs_obd_cleanup(obd); + RETURN(0); } diff --git a/lustre/lov/lov_pack.c b/lustre/lov/lov_pack.c index 0165f69..7fd5470 100644 --- a/lustre/lov/lov_pack.c +++ b/lustre/lov/lov_pack.c @@ -484,8 +484,7 @@ static int __lov_setstripe(struct obd_export *exp, struct lov_stripe_md **lsmp, rc = lov_check_index_in_pool(lumv3.lmm_stripe_offset, pool); if (rc < 0) { - lh_put(lov->lov_pools_hash_body, - &pool->pool_hash); + lov_pool_putref(pool); RETURN(-EINVAL); } } @@ -493,7 +492,7 @@ static int __lov_setstripe(struct obd_export *exp, struct lov_stripe_md **lsmp, if (stripe_count > pool_tgt_count(pool)) stripe_count = pool_tgt_count(pool); - lh_put(lov->lov_pools_hash_body, &pool->pool_hash); + lov_pool_putref(pool); } if ((__u64)lumv1->lmm_stripe_size * stripe_count > ~0UL) { diff --git a/lustre/lov/lov_pool.c b/lustre/lov/lov_pool.c index 764a494..3df37a7 100644 --- a/lustre/lov/lov_pool.c +++ b/lustre/lov/lov_pool.c @@ -38,6 +38,8 @@ * OST pool methods * * Author: Jacques-Charles LAFOUCRIERE + * Author: Alex Lyashkov + * Author: Nathaniel Rutman */ #define DEBUG_SUBSYSTEM S_LOV @@ -51,15 +53,23 @@ #include #include "lov_internal.h" -static void lov_pool_getref(struct pool_desc *pool) { +static void lov_pool_getref(struct pool_desc *pool) +{ + CDEBUG(D_INFO, "pool %p\n", pool); atomic_inc(&pool->pool_refcount); } -static void lov_pool_putref(struct pool_desc *pool) { +void lov_pool_putref(struct pool_desc *pool) +{ + CDEBUG(D_INFO, "pool %p\n", pool); if (atomic_dec_and_test(&pool->pool_refcount)) { + LASSERT(hlist_unhashed(&pool->pool_hash)); + LASSERT(list_empty(&pool->pool_list)); + LASSERT(pool->pool_proc_entry == NULL); lov_ost_pool_free(&(pool->pool_rr.lqr_pool)); lov_ost_pool_free(&(pool->pool_obds)); OBD_FREE_PTR(pool); + EXIT; } } @@ -302,6 +312,8 @@ void lov_dump_pool(int level, struct pool_desc *pool) #define LOV_POOL_INIT_COUNT 2 int lov_ost_pool_init(struct ost_pool *op, unsigned int count) { + ENTRY; + if (count == 0) count = LOV_POOL_INIT_COUNT; op->op_array = NULL; @@ -311,8 +323,9 @@ int lov_ost_pool_init(struct ost_pool *op, unsigned int count) OBD_ALLOC(op->op_array, op->op_size * sizeof(op->op_array[0])); if (op->op_array == NULL) { op->op_size = 0; - return -ENOMEM; + RETURN(-ENOMEM); } + EXIT; return 0; } @@ -359,6 +372,7 @@ int lov_ost_pool_add(struct ost_pool *op, __u32 idx, unsigned int min_count) /* ost not found we add it */ op->op_array[op->op_count] = idx; op->op_count++; + EXIT; out: up_write(&op->op_rw_sem); return rc; @@ -367,6 +381,7 @@ out: int lov_ost_pool_remove(struct ost_pool *op, __u32 idx) { int i; + ENTRY; down_write(&op->op_rw_sem); @@ -376,18 +391,21 @@ int lov_ost_pool_remove(struct ost_pool *op, __u32 idx) (op->op_count - i - 1) * sizeof(op->op_array[0])); op->op_count--; up_write(&op->op_rw_sem); + EXIT; return 0; } } up_write(&op->op_rw_sem); - return -EINVAL; + RETURN(-EINVAL); } int lov_ost_pool_free(struct ost_pool *op) { + ENTRY; + if (op->op_size == 0) - return 0; + RETURN(0); down_write(&op->op_rw_sem); @@ -397,7 +415,7 @@ int lov_ost_pool_free(struct ost_pool *op) op->op_size = 0; up_write(&op->op_rw_sem); - return 0; + RETURN(0); } @@ -430,48 +448,54 @@ int lov_pool_new(struct obd_device *obd, char *poolname) memset(&(new_pool->pool_rr), 0, sizeof(struct lov_qos_rr)); rc = lov_ost_pool_init(&new_pool->pool_rr.lqr_pool, 0); - if (rc) { - lov_ost_pool_free(&new_pool->pool_obds); - GOTO(out_err, rc); - } + if (rc) + GOTO(out_free_pool_obds, rc); INIT_HLIST_NODE(&new_pool->pool_hash); - rc = lustre_hash_add_unique(lov->lov_pools_hash_body, poolname, - &new_pool->pool_hash); - if (rc) { - lov_ost_pool_free(&new_pool->pool_rr.lqr_pool); - lov_ost_pool_free(&new_pool->pool_obds); - GOTO(out_err, rc = -EEXIST); - } - - spin_lock(&obd->obd_dev_lock); - list_add_tail(&new_pool->pool_list, &lov->lov_pool_list); - lov->lov_pool_count++; - - spin_unlock(&obd->obd_dev_lock); - - CDEBUG(D_CONFIG, LOV_POOLNAMEF" is pool #%d\n", - poolname, lov->lov_pool_count); #ifdef LPROCFS - /* ifdef needed for liblustre */ + /* we need this assert seq_file is not implementated for liblustre */ /* get ref for /proc file */ lov_pool_getref(new_pool); new_pool->pool_proc_entry = lprocfs_add_simple(lov->lov_pool_proc_entry, poolname, NULL, NULL, new_pool, &pool_proc_operations); -#endif - if (IS_ERR(new_pool->pool_proc_entry)) { CWARN("Cannot add proc pool entry "LOV_POOLNAMEF"\n", poolname); new_pool->pool_proc_entry = NULL; lov_pool_putref(new_pool); } + CDEBUG(D_INFO, "pool %p - proc %p\n", new_pool, new_pool->pool_proc_entry); +#endif + + spin_lock(&obd->obd_dev_lock); + list_add_tail(&new_pool->pool_list, &lov->lov_pool_list); + lov->lov_pool_count++; + spin_unlock(&obd->obd_dev_lock); + + /* add to find only when it fully ready */ + rc = lustre_hash_add_unique(lov->lov_pools_hash_body, poolname, + &new_pool->pool_hash); + if (rc) + GOTO(out_err, rc = -EEXIST); + + CDEBUG(D_CONFIG, LOV_POOLNAMEF" is pool #%d\n", + poolname, lov->lov_pool_count); RETURN(0); out_err: + spin_lock(&obd->obd_dev_lock); + list_del_init(&new_pool->pool_list); + lov->lov_pool_count--; + spin_unlock(&obd->obd_dev_lock); + + lprocfs_remove(&new_pool->pool_proc_entry); + + lov_ost_pool_free(&new_pool->pool_rr.lqr_pool); +out_free_pool_obds: + lov_ost_pool_free(&new_pool->pool_obds); OBD_FREE_PTR(new_pool); return rc; } @@ -484,33 +508,23 @@ int lov_pool_del(struct obd_device *obd, char *poolname) lov = &(obd->u.lov); - spin_lock(&obd->obd_dev_lock); - - pool = lustre_hash_lookup(lov->lov_pools_hash_body, poolname); - if (pool == NULL) { - spin_unlock(&obd->obd_dev_lock); + /* lookup and kill hash reference */ + pool = lustre_hash_del_key(lov->lov_pools_hash_body, poolname); + if (pool == NULL) RETURN(-ENOENT); - } -#ifdef LPROCFS if (pool->pool_proc_entry != NULL) { - remove_proc_entry(pool->pool_proc_entry->name, - pool->pool_proc_entry->parent); - /* remove ref for /proc file */ + CDEBUG(D_INFO, "proc entry %p\n", pool->pool_proc_entry); + lprocfs_remove(&pool->pool_proc_entry); lov_pool_putref(pool); } -#endif - lustre_hash_del_key(lov->lov_pools_hash_body, poolname); + spin_lock(&obd->obd_dev_lock); list_del_init(&pool->pool_list); - lov->lov_pool_count--; - lh_put(lov->lov_pools_hash_body, &pool->pool_hash); spin_unlock(&obd->obd_dev_lock); - /* remove ref got when pool was created in memory - * pool will be freed when refount will reach 0 - */ + /* release last reference */ lov_pool_putref(pool); RETURN(0); @@ -522,7 +536,7 @@ int lov_pool_add(struct obd_device *obd, char *poolname, char *ostname) struct obd_uuid ost_uuid; struct lov_obd *lov; struct pool_desc *pool; - unsigned int i, lov_idx; + unsigned int lov_idx; int rc; ENTRY; @@ -536,22 +550,17 @@ int lov_pool_add(struct obd_device *obd, char *poolname, char *ostname) /* search ost in lov array */ - mutex_down(&lov->lov_lock); - for (i = 0; i < lov->desc.ld_tgt_count; i++) { - if (!lov->lov_tgts[i]) + lov_getref(obd); + for (lov_idx = 0; lov_idx < lov->desc.ld_tgt_count; lov_idx++) { + if (!lov->lov_tgts[lov_idx]) continue; - if (obd_uuid_equals(&ost_uuid, &(lov->lov_tgts[i]->ltd_uuid))) + if (obd_uuid_equals(&ost_uuid, + &(lov->lov_tgts[lov_idx]->ltd_uuid))) break; } - /* test if ost found in lov */ - if (i == lov->desc.ld_tgt_count) { - mutex_up(&lov->lov_lock); + if (lov_idx == lov->desc.ld_tgt_count) GOTO(out, rc = -EINVAL); - } - mutex_up(&lov->lov_lock); - - lov_idx = i; rc = lov_ost_pool_add(&pool->pool_obds, lov_idx, lov->lov_tgt_size); if (rc) @@ -564,7 +573,8 @@ int lov_pool_add(struct obd_device *obd, char *poolname, char *ostname) EXIT; out: - lh_put(lov->lov_pools_hash_body, &pool->pool_hash); + lov_putref(obd); + lov_pool_putref(pool); return rc; } @@ -573,39 +583,32 @@ int lov_pool_remove(struct obd_device *obd, char *poolname, char *ostname) struct obd_uuid ost_uuid; struct lov_obd *lov; struct pool_desc *pool; - unsigned int i, lov_idx; + unsigned int lov_idx; int rc = 0; ENTRY; lov = &(obd->u.lov); - spin_lock(&obd->obd_dev_lock); pool = lustre_hash_lookup(lov->lov_pools_hash_body, poolname); - if (pool == NULL) { - spin_unlock(&obd->obd_dev_lock); + if (pool == NULL) RETURN(-ENOENT); - } obd_str2uuid(&ost_uuid, ostname); + lov_getref(obd); /* search ost in lov array, to get index */ - for (i = 0; i < lov->desc.ld_tgt_count; i++) { - if (!lov->lov_tgts[i]) + for (lov_idx = 0; lov_idx < lov->desc.ld_tgt_count; lov_idx++) { + if (!lov->lov_tgts[lov_idx]) continue; - if (obd_uuid_equals(&ost_uuid, &(lov->lov_tgts[i]->ltd_uuid))) + if (obd_uuid_equals(&ost_uuid, + &(lov->lov_tgts[lov_idx]->ltd_uuid))) break; } /* test if ost found in lov */ - if (i == lov->desc.ld_tgt_count) { - spin_unlock(&obd->obd_dev_lock); + if (lov_idx == lov->desc.ld_tgt_count) GOTO(out, rc = -EINVAL); - } - - spin_unlock(&obd->obd_dev_lock); - - lov_idx = i; lov_ost_pool_remove(&pool->pool_obds, lov_idx); @@ -616,7 +619,8 @@ int lov_pool_remove(struct obd_device *obd, char *poolname, char *ostname) EXIT; out: - lh_put(lov->lov_pools_hash_body, &pool->pool_hash); + lov_putref(obd); + lov_pool_putref(pool); return rc; } @@ -660,7 +664,7 @@ struct pool_desc *lov_find_pool(struct lov_obd *lov, char *poolname) CWARN("Request for an empty pool ("LOV_POOLNAMEF")\n", poolname); /* pool is ignored, so we remove ref on it */ - lh_put(lov->lov_pools_hash_body, &pool->pool_hash); + lov_pool_putref(pool); pool = NULL; } } diff --git a/lustre/lov/lov_qos.c b/lustre/lov/lov_qos.c index d020dd7..94539b3 100644 --- a/lustre/lov/lov_qos.c +++ b/lustre/lov/lov_qos.c @@ -637,7 +637,7 @@ out: if (pool != NULL) { up_read(&pool_tgt_rw_sem(pool)); /* put back ref got by lov_find_pool() */ - lh_put(lov->lov_pools_hash_body, &pool->pool_hash); + lov_pool_putref(pool); } RETURN(rc); @@ -729,7 +729,7 @@ out: if (pool != NULL) { up_read(&pool_tgt_rw_sem(pool)); /* put back ref got by lov_find_pool() */ - lh_put(lov->lov_pools_hash_body, &pool->pool_hash); + lov_pool_putref(pool); } RETURN(rc); @@ -924,7 +924,7 @@ out_nolock: if (pool != NULL) { up_read(&pool_tgt_rw_sem(pool)); /* put back ref got by lov_find_pool() */ - lh_put(lov->lov_pools_hash_body, &pool->pool_hash); + lov_pool_putref(pool); } if (rc == -EAGAIN) diff --git a/lustre/ptlrpc/recov_thread.c b/lustre/ptlrpc/recov_thread.c index dc4b13b..e90142b 100644 --- a/lustre/ptlrpc/recov_thread.c +++ b/lustre/ptlrpc/recov_thread.c @@ -591,6 +591,7 @@ int llog_obd_repl_cancel(struct llog_ctxt *ctxt, mutex_down(&ctxt->loc_sem); lcm = ctxt->loc_lcm; + CDEBUG(D_INFO, "cancel on lsm %p\n", lcm); /* * Let's check if we have all structures alive. We also check for