Whamcloud - gitweb
LU-106 procfs: many proc entries are not accessed safely
authorLai Siyao <laisiyao@whamcloud.com>
Mon, 13 Jun 2011 03:56:03 +0000 (20:56 -0700)
committerOleg Drokin <green@whamcloud.com>
Mon, 12 Dec 2011 21:41:47 +0000 (16:41 -0500)
Some in memory data may be released/uninitialized at the time
of proc entry creation/removal, this patch includes the following
fixes:
* initialize data before proc entry creation
* free data after proc entry removal
* free proc entries in obd_precleanup() because
  obd_uuid/nid/nid_stats_hash are released in class_cleanup().
* free proc entries after obd_zombie_barrier() because obd_export
  hold one refcound of nid_stat.
* check osd->od_mount before accessing osd proc entries because the
  osd proc entries are created before mount.

Signed-off-by: Lai Siyao <laisiyao@whamcloud.com>
Change-Id: I03cb977e1be0747032a70f6a39fec804f81d70cc
Reviewed-on: http://review.whamcloud.com/326
Tested-by: Hudson
Reviewed-by: Johann Lombardi <johann@whamcloud.com>
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
15 files changed:
lustre/cmm/cmm_device.c
lustre/fld/fld_request.c
lustre/include/lprocfs_status.h
lustre/include/lustre_fld.h
lustre/lmv/lmv_obd.c
lustre/lov/lov_obd.c
lustre/mdc/mdc_request.c
lustre/mdt/mdt_handler.c
lustre/mdt/mdt_lproc.c
lustre/mgs/mgs_handler.c
lustre/obdclass/lprocfs_status.c
lustre/obdclass/obd_config.c
lustre/obdfilter/filter.c
lustre/osc/osc_request.c
lustre/osd-ldiskfs/osd_lproc.c

index 2105222..e17358d 100644 (file)
@@ -837,6 +837,7 @@ static struct lu_device *cmm_device_fini(const struct lu_env *env,
         }
         cfs_spin_unlock(&cm->cmm_tgt_guard);
 
+        fld_client_proc_fini(cm->cmm_fld);
         fld_client_fini(cm->cmm_fld);
         ls = cmm2lu_dev(cm)->ld_site;
         lu_site2md(ls)->ms_client_fld = NULL;
index 2ea527a..af34ec4 100644 (file)
@@ -280,8 +280,6 @@ int fld_client_del_target(struct lu_client_fld *fld,
 }
 EXPORT_SYMBOL(fld_client_del_target);
 
-static void fld_client_proc_fini(struct lu_client_fld *fld);
-
 #ifdef LPROCFS
 static int fld_client_proc_init(struct lu_client_fld *fld)
 {
@@ -314,7 +312,7 @@ out_cleanup:
         return rc;
 }
 
-static void fld_client_proc_fini(struct lu_client_fld *fld)
+void fld_client_proc_fini(struct lu_client_fld *fld)
 {
         ENTRY;
         if (fld->lcf_proc_dir) {
@@ -330,12 +328,14 @@ static int fld_client_proc_init(struct lu_client_fld *fld)
         return 0;
 }
 
-static void fld_client_proc_fini(struct lu_client_fld *fld)
+void fld_client_proc_fini(struct lu_client_fld *fld)
 {
         return;
 }
 #endif
 
+EXPORT_SYMBOL(fld_client_proc_fini);
+
 static inline int hash_is_sane(int hash)
 {
         return (hash >= 0 && hash < ARRAY_SIZE(fld_hash));
@@ -398,8 +398,6 @@ void fld_client_fini(struct lu_client_fld *fld)
         struct lu_fld_target *target, *tmp;
         ENTRY;
 
-        fld_client_proc_fini(fld);
-
         cfs_spin_lock(&fld->lcf_lock);
         cfs_list_for_each_entry_safe(target, tmp,
                                      &fld->lcf_targets, ft_chain) {
index aeed486..09bf178 100644 (file)
@@ -486,7 +486,6 @@ extern cfs_proc_dir_entry_t *lprocfs_srch(cfs_proc_dir_entry_t *root,
 
 extern int lprocfs_obd_setup(struct obd_device *obd, struct lprocfs_vars *list);
 extern int lprocfs_obd_cleanup(struct obd_device *obd);
-extern void lprocfs_free_per_client_stats(struct obd_device *obd);
 extern struct file_operations lprocfs_evict_client_fops;
 
 extern int lprocfs_seq_create(cfs_proc_dir_entry_t *parent, char *name,
index 428b352..a0328d1 100644 (file)
@@ -193,6 +193,8 @@ int fld_client_add_target(struct lu_client_fld *fld,
 int fld_client_del_target(struct lu_client_fld *fld,
                           __u64 idx);
 
+void fld_client_proc_fini(struct lu_client_fld *fld);
+
 /** @} fld */
 
 #endif
index 4aef69a..3e765ba 100644 (file)
@@ -1140,7 +1140,6 @@ static int lmv_cleanup(struct obd_device *obd)
         ENTRY;
 
         fld_client_fini(&lmv->lmv_fld);
-        lprocfs_obd_cleanup(obd);
         lmv_object_cleanup(obd);
         OBD_FREE(lmv->datas, lmv->datas_size);
         OBD_FREE(lmv->tgts, lmv->tgts_size);
@@ -2631,7 +2630,8 @@ repeat:
 
 static int lmv_precleanup(struct obd_device *obd, enum obd_cleanup_stage stage)
 {
-        int        rc = 0;
+        struct lmv_obd *lmv = &obd->u.lmv;
+        int rc = 0;
 
         switch (stage) {
         case OBD_CLEANUP_EARLY:
@@ -2639,6 +2639,8 @@ static int lmv_precleanup(struct obd_device *obd, enum obd_cleanup_stage stage)
                  * stack. */
                 break;
         case OBD_CLEANUP_EXPORTS:
+                fld_client_proc_fini(&lmv->lmv_fld);
+                lprocfs_obd_cleanup(obd);
                 rc = obd_llog_finish(obd, 0);
                 if (rc != 0)
                         CERROR("failed to cleanup llogging subsystems\n");
index 0497cb2..a70e64a 100644 (file)
@@ -884,6 +884,7 @@ static int lov_precleanup(struct obd_device *obd, enum obd_cleanup_stage stage)
                 break;
         }
         case OBD_CLEANUP_EXPORTS:
+                lprocfs_obd_cleanup(obd);
                 rc = obd_llog_finish(obd, 0);
                 if (rc != 0)
                         CERROR("failed to cleanup llogging subsystems\n");
@@ -897,6 +898,7 @@ static int lov_cleanup(struct obd_device *obd)
         struct lov_obd *lov = &obd->u.lov;
         cfs_list_t *pos, *tmp;
         struct pool_desc *pool;
+        ENTRY;
 
         cfs_list_for_each_safe(pos, tmp, &lov->lov_pool_list) {
                 pool = cfs_list_entry(pos, struct pool_desc, pool_list);
@@ -932,10 +934,6 @@ static int lov_cleanup(struct obd_device *obd)
                          lov->lov_tgt_size);
                 lov->lov_tgt_size = 0;
         }
-
-        /* clear pools parent proc entry only after all pools is killed */
-        lprocfs_obd_cleanup(obd);
-
         OBD_FREE_PTR(lov->lov_qos.lq_statfs_data);
         RETURN(0);
 }
index 9b63d42..2f05091 100644 (file)
@@ -2082,6 +2082,8 @@ static int mdc_precleanup(struct obd_device *obd, enum obd_cleanup_stage stage)
                         libcfs_kkuc_group_rem(0, KUC_GRP_HSM);
 
                 obd_cleanup_client_import(obd);
+                ptlrpc_lprocfs_unregister_obd(obd);
+                lprocfs_obd_cleanup(obd);
 
                 rc = obd_llog_finish(obd, 0);
                 if (rc != 0)
@@ -2099,8 +2101,6 @@ static int mdc_cleanup(struct obd_device *obd)
         OBD_FREE(cli->cl_setattr_lock, sizeof (*cli->cl_setattr_lock));
         OBD_FREE(cli->cl_close_lock, sizeof (*cli->cl_close_lock));
 
-        ptlrpc_lprocfs_unregister_obd(obd);
-        lprocfs_obd_cleanup(obd);
         ptlrpcd_decref();
 
         return client_obd_cleanup(obd);
index 001ef1d..8a57ab0 100644 (file)
@@ -4319,6 +4319,9 @@ static void mdt_fini(const struct lu_env *env, struct mdt_device *m)
         mdt_obd_llog_cleanup(obd);
         obd_exports_barrier(obd);
         obd_zombie_barrier();
+
+        mdt_procfs_fini(m);
+
 #ifdef HAVE_QUOTA_SUPPORT
         next->md_ops->mdo_quota.mqo_cleanup(env, next);
 #endif
@@ -4354,10 +4357,6 @@ static void mdt_fini(const struct lu_env *env, struct mdt_device *m)
          */
         mdt_stack_fini(env, m, md2lu_dev(m->mdt_child));
 
-        lprocfs_free_per_client_stats(obd);
-        lprocfs_free_obd_stats(obd);
-        mdt_procfs_fini(m);
-
         if (ls) {
                 struct md_site *mite;
 
@@ -4492,12 +4491,6 @@ static int mdt_init0(const struct lu_env *env, struct mdt_device *m,
                 GOTO(err_free_site, rc);
         }
 
-        rc = mdt_procfs_init(m, dev);
-        if (rc) {
-                CERROR("Can't init MDT lprocfs, rc %d\n", rc);
-                GOTO(err_fini_proc, rc);
-        }
-
         /* set server index */
         lu_site2md(s)->ms_node_id = node_id;
 
@@ -4521,7 +4514,7 @@ static int mdt_init0(const struct lu_env *env, struct mdt_device *m,
         rc = mdt_stack_init((struct lu_env *)env, m, cfg, lmi);
         if (rc) {
                 CERROR("Can't init device stack, rc %d\n", rc);
-                GOTO(err_fini_proc, rc);
+                GOTO(err_lu_site, rc);
         }
 
         rc = lut_init(env, &m->mdt_lut, obd, m->mdt_bottom);
@@ -4609,9 +4602,15 @@ static int mdt_init0(const struct lu_env *env, struct mdt_device *m,
 
         target_recovery_init(&m->mdt_lut, mdt_recovery_handle);
 
+        rc = mdt_procfs_init(m, dev);
+        if (rc) {
+                CERROR("Can't init MDT lprocfs, rc %d\n", rc);
+                GOTO(err_recovery, rc);
+        }
+
         rc = mdt_start_ptlrpc_service(m);
         if (rc)
-                GOTO(err_recovery, rc);
+                GOTO(err_procfs, rc);
 
         ping_evictor_start();
 
@@ -4635,6 +4634,8 @@ static int mdt_init0(const struct lu_env *env, struct mdt_device *m,
 err_stop_service:
         ping_evictor_stop();
         mdt_stop_ptlrpc_service(m);
+err_procfs:
+        mdt_procfs_fini(m);
 err_recovery:
         target_recovery_fini(obd);
         upcall_cache_cleanup(m->mdt_identity_cache);
@@ -4662,8 +4663,7 @@ err_lut:
         lut_fini(env, &m->mdt_lut);
 err_fini_stack:
         mdt_stack_fini(env, m, md2lu_dev(m->mdt_child));
-err_fini_proc:
-        mdt_procfs_fini(m);
+err_lu_site:
         lu_site_fini(s);
 err_free_site:
         OBD_FREE_PTR(mite);
index 053f626..f141398 100644 (file)
@@ -126,18 +126,20 @@ int mdt_procfs_fini(struct mdt_device *mdt)
         struct lu_device *ld = &mdt->mdt_md_dev.md_lu_dev;
         struct obd_device *obd = ld->ld_obd;
 
-        if (mdt->mdt_proc_entry) {
-                lu_time_fini(&ld->ld_site->ls_time_stats);
-                lu_time_fini(&mdt->mdt_stats);
-                mdt->mdt_proc_entry = NULL;
-        }
         if (obd->obd_proc_exports_entry) {
                 lprocfs_remove_proc_entry("clear", obd->obd_proc_exports_entry);
                 obd->obd_proc_exports_entry = NULL;
         }
+        lprocfs_free_per_client_stats(obd);
+        lprocfs_obd_cleanup(obd);
         ptlrpc_lprocfs_unregister_obd(obd);
+        if (mdt->mdt_proc_entry) {
+                lu_time_fini(&ld->ld_site->ls_time_stats);
+                lu_time_fini(&mdt->mdt_stats);
+                mdt->mdt_proc_entry = NULL;
+        }
         lprocfs_free_md_stats(obd);
-        lprocfs_obd_cleanup(obd);
+        lprocfs_free_obd_stats(obd);
 
         RETURN(0);
 }
@@ -536,7 +538,6 @@ static int lprocfs_rd_root_squash(char *page, char **start, off_t off,
 {
         struct obd_device *obd = data;
         struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev);
-        ENTRY;
 
         return snprintf(page, count, "%u:%u\n", mdt->mdt_squash_uid,
                         mdt->mdt_squash_gid);
index bb2705e..e8b1d38 100644 (file)
@@ -286,13 +286,19 @@ err_put:
 
 static int mgs_precleanup(struct obd_device *obd, enum obd_cleanup_stage stage)
 {
+        struct mgs_obd *mgs = &obd->u.mgs;
         int rc = 0;
         ENTRY;
 
         switch (stage) {
         case OBD_CLEANUP_EARLY:
+                break;
         case OBD_CLEANUP_EXPORTS:
+                ping_evictor_stop();
+                ptlrpc_unregister_service(mgs->mgs_service);
+                mgs_cleanup_fsdb_list(obd);
                 rc = obd_llog_finish(obd, 0);
+                lproc_mgs_cleanup(obd);
                 break;
         }
         RETURN(rc);
@@ -309,12 +315,6 @@ static int mgs_cleanup(struct obd_device *obd)
         if (mgs->mgs_sb == NULL)
                 RETURN(0);
 
-        ping_evictor_stop();
-
-        ptlrpc_unregister_service(mgs->mgs_service);
-
-        mgs_cleanup_fsdb_list(obd);
-        lproc_mgs_cleanup(obd);
         mgs_fs_cleanup(obd);
 
         server_put_mount(obd->obd_name, mgs->mgs_vfsmnt);
index 1610794..d85954b 100644 (file)
@@ -1146,8 +1146,6 @@ static void lprocfs_free_client_stats(struct nid_stat *client_stat)
                  "nid %s:count %d\n", libcfs_nid2str(client_stat->nid),
                  atomic_read(&client_stat->nid_exp_ref_count));
 
-        cfs_hlist_del_init(&client_stat->nid_hash);
-
         if (client_stat->nid_proc)
                 lprocfs_remove(&client_stat->nid_proc);
 
@@ -1167,18 +1165,19 @@ static void lprocfs_free_client_stats(struct nid_stat *client_stat)
 
 void lprocfs_free_per_client_stats(struct obd_device *obd)
 {
+        cfs_hash_t *hash = obd->obd_nid_stats_hash;
         struct nid_stat *stat;
         ENTRY;
 
         /* we need extra list - because hash_exit called to early */
         /* not need locking because all clients is died */
-        while(!cfs_list_empty(&obd->obd_nid_stats)) {
+        while (!cfs_list_empty(&obd->obd_nid_stats)) {
                 stat = cfs_list_entry(obd->obd_nid_stats.next,
                                       struct nid_stat, nid_list);
                 cfs_list_del_init(&stat->nid_list);
+                cfs_hash_del(hash, &stat->nid, &stat->nid_hash);
                 lprocfs_free_client_stats(stat);
         }
-
         EXIT;
 }
 
@@ -1778,7 +1777,7 @@ int lprocfs_nid_stats_clear_read(char *page, char **start, off_t off,
 }
 EXPORT_SYMBOL(lprocfs_nid_stats_clear_read);
 
-int lprocfs_nid_stats_clear_write_cb(void *obj, void *data)
+static int lprocfs_nid_stats_clear_write_cb(void *obj, void *data)
 {
         struct nid_stat *stat = obj;
         int i;
index f9fa331..4c2cf37 100644 (file)
@@ -615,6 +615,12 @@ int class_cleanup(struct obd_device *obd, struct lustre_cfg *lcfg)
                 class_disconnect_exports(obd);
         }
 
+        /* Precleanup, we must make sure all exports get destroyed. */
+        err = obd_precleanup(obd, OBD_CLEANUP_EXPORTS);
+        if (err)
+                CERROR("Precleanup %s returned %d\n",
+                       obd->obd_name, err);
+
         /* destroy an uuid-export hash body */
         if (obd->obd_uuid_hash) {
                 cfs_hash_putref(obd->obd_uuid_hash);
@@ -633,13 +639,9 @@ int class_cleanup(struct obd_device *obd, struct lustre_cfg *lcfg)
                 obd->obd_nid_stats_hash = NULL;
         }
 
-        /* Precleanup, we must make sure all exports get destroyed. */
-        err = obd_precleanup(obd, OBD_CLEANUP_EXPORTS);
-        if (err)
-                CERROR("Precleanup %s returned %d\n",
-                       obd->obd_name, err);
         class_decref(obd, "setup", obd);
         obd->obd_set_up = 0;
+
         RETURN(0);
 }
 
index 31f4cbe..a77ca65 100644 (file)
@@ -2611,7 +2611,16 @@ static int filter_precleanup(struct obd_device *obd,
         case OBD_CLEANUP_EXPORTS:
                 /* Stop recovery before namespace cleanup. */
                 target_recovery_fini(obd);
+
+                obd_exports_barrier(obd);
+                obd_zombie_barrier();
+
                 rc = filter_llog_preclean(obd);
+                lprocfs_remove_proc_entry("clear", obd->obd_proc_exports_entry);
+                lprocfs_free_per_client_stats(obd);
+                lprocfs_obd_cleanup(obd);
+                lprocfs_free_obd_stats(obd);
+                lquota_cleanup(filter_quota_interface_ref, obd);
                 break;
         }
         RETURN(rc);
@@ -2626,15 +2635,6 @@ static int filter_cleanup(struct obd_device *obd)
                 LCONSOLE_WARN("%s: shutting down for failover; client state "
                               "will be preserved.\n", obd->obd_name);
 
-        obd_exports_barrier(obd);
-        obd_zombie_barrier();
-
-        lprocfs_remove_proc_entry("clear", obd->obd_proc_exports_entry);
-        lprocfs_free_per_client_stats(obd);
-        lprocfs_free_obd_stats(obd);
-        lprocfs_obd_cleanup(obd);
-        lquota_cleanup(filter_quota_interface_ref, obd);
-
         ldlm_namespace_free(obd->obd_namespace, NULL, obd->obd_force);
         obd->obd_namespace = NULL;
 
index a0d45b5..c5e4bd0 100644 (file)
@@ -4533,6 +4533,8 @@ static int osc_precleanup(struct obd_device *obd, enum obd_cleanup_stage stage)
                  */
                 obd_zombie_barrier();
                 obd_cleanup_client_import(obd);
+                ptlrpc_lprocfs_unregister_obd(obd);
+                lprocfs_obd_cleanup(obd);
                 rc = obd_llog_finish(obd, 0);
                 if (rc != 0)
                         CERROR("failed to cleanup llogging subsystems\n");
@@ -4547,8 +4549,6 @@ int osc_cleanup(struct obd_device *obd)
         int rc;
 
         ENTRY;
-        ptlrpc_lprocfs_unregister_obd(obd);
-        lprocfs_obd_cleanup(obd);
 
         /* free memory of osc quota cache */
         lquota_cleanup(quota_interface, obd);
index df25006..a8862e6 100644 (file)
@@ -121,7 +121,12 @@ int lprocfs_osd_rd_blksize(char *page, char **start, off_t off, int count,
                            int *eof, void *data)
 {
         struct osd_device *osd = data;
-        int rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
+        int rc;
+
+        if (unlikely(osd->od_mount == NULL))
+                return -EINPROGRESS;
+
+        rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
         if (!rc) {
                 *eof = 1;
                 rc = snprintf(page, count, "%ld\n", osd->od_kstatfs.f_bsize);
@@ -133,7 +138,12 @@ int lprocfs_osd_rd_kbytestotal(char *page, char **start, off_t off, int count,
                                int *eof, void *data)
 {
         struct osd_device *osd = data;
-        int rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
+        int rc;
+
+        if (unlikely(osd->od_mount == NULL))
+                return -EINPROGRESS;
+
+        rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
         if (!rc) {
                 __u32 blk_size = osd->od_kstatfs.f_bsize >> 10;
                 __u64 result = osd->od_kstatfs.f_blocks;
@@ -151,7 +161,12 @@ int lprocfs_osd_rd_kbytesfree(char *page, char **start, off_t off, int count,
                               int *eof, void *data)
 {
         struct osd_device *osd = data;
-        int rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
+        int rc;
+
+        if (unlikely(osd->od_mount == NULL))
+                return -EINPROGRESS;
+
+        rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
         if (!rc) {
                 __u32 blk_size = osd->od_kstatfs.f_bsize >> 10;
                 __u64 result = osd->od_kstatfs.f_bfree;
@@ -169,7 +184,12 @@ int lprocfs_osd_rd_kbytesavail(char *page, char **start, off_t off, int count,
                                int *eof, void *data)
 {
         struct osd_device *osd = data;
-        int rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
+        int rc;
+
+        if (unlikely(osd->od_mount == NULL))
+                return -EINPROGRESS;
+
+        rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
         if (!rc) {
                 __u32 blk_size = osd->od_kstatfs.f_bsize >> 10;
                 __u64 result = osd->od_kstatfs.f_bavail;
@@ -187,7 +207,12 @@ int lprocfs_osd_rd_filestotal(char *page, char **start, off_t off, int count,
                               int *eof, void *data)
 {
         struct osd_device *osd = data;
-        int rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
+        int rc;
+
+        if (unlikely(osd->od_mount == NULL))
+                return -EINPROGRESS;
+
+        rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
         if (!rc) {
                 *eof = 1;
                 rc = snprintf(page, count, LPU64"\n", osd->od_kstatfs.f_files);
@@ -200,7 +225,12 @@ int lprocfs_osd_rd_filesfree(char *page, char **start, off_t off, int count,
                              int *eof, void *data)
 {
         struct osd_device *osd = data;
-        int rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
+        int rc;
+
+        if (unlikely(osd->od_mount == NULL))
+                return -EINPROGRESS;
+
+        rc = osd_statfs(NULL, &osd->od_dt_dev, &osd->od_kstatfs);
         if (!rc) {
                 *eof = 1;
                 rc = snprintf(page, count, LPU64"\n", osd->od_kstatfs.f_ffree);
@@ -223,6 +253,9 @@ static int lprocfs_osd_rd_mntdev(char *page, char **start, off_t off, int count,
         struct osd_device *osd = data;
 
         LASSERT(osd != NULL);
+        if (unlikely(osd->od_mount == NULL))
+                return -EINPROGRESS;
+
         LASSERT(osd->od_mount->lmi_mnt->mnt_devname);
         *eof = 1;