From 89aff2f3a17d26f9b86f9afd1bd4631358586ce5 Mon Sep 17 00:00:00 2001 From: Mr NeilBrown Date: Thu, 2 Jan 2020 19:50:55 -0500 Subject: [PATCH] LU-9679 llite: fix possible race with module unload. lustre_fill_super() calls client_fill_super() without holding a reference to the module containing client_fill_super. If that module is unloaded at a bad time, this can crash. To be able to get a reference to the module using try_get_module(), we need a pointer to the module. So replace lustre_register_client_fill_super() and lustre_register_kill_super_cb() with a single lustre_register_super_ops() which also passed a module pointer. Then use a spinlock to ensure the module pointer isn't removed while try_module_get() is running, and use try_module_get() to ensure we have a reference before calling client_fill_super(). Now that we take the reference to the module before calling luster_fill_super(), we don't need to take one inside lustre_fill_super(). Linux-commit: d487fe31f49e78f3cdd826923bf0c340a839ffd8 Signed-off-by: Mr NeilBrown Change-Id: I9474622f2a253d9882eae3f0578c50782dd11ad4 Reviewed-on: https://review.whamcloud.com/37020 Tested-by: jenkins Tested-by: Maloo Reviewed-by: James Simmons Reviewed-by: Jian Yu Reviewed-by: Petros Koutoupis Reviewed-by: Oleg Drokin --- lustre/include/lustre_disk.h | 5 +++-- lustre/llite/llite_lib.c | 2 -- lustre/llite/super25.c | 6 ++---- lustre/obdclass/obd_mount.c | 32 +++++++++++++++++++++----------- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/lustre/include/lustre_disk.h b/lustre/include/lustre_disk.h index a809f4b..385cc1e9 100644 --- a/lustre/include/lustre_disk.h +++ b/lustre/include/lustre_disk.h @@ -349,8 +349,9 @@ int lustre_start_mgc(struct super_block *sb); int server_name2fsname(const char *svname, char *fsname, const char **endptr); void obdname2fsname(const char *tgt, char *fsname, size_t fslen); -void lustre_register_client_fill_super(int (*cfs)(struct super_block *sb)); -void lustre_register_kill_super_cb(void (*cfs)(struct super_block *sb)); +void lustre_register_super_ops(struct module *mod, + int (*cfs)(struct super_block *sb), + void (*ksc)(struct super_block *sb)); int lustre_common_put_super(struct super_block *sb); int mgc_fsname2resid(char *fsname, struct ldlm_res_id *res_id, int type); diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c index f92cd7d..69a50629 100644 --- a/lustre/llite/llite_lib.c +++ b/lustre/llite/llite_lib.c @@ -1075,8 +1075,6 @@ int ll_fill_super(struct super_block *sb) CDEBUG(D_VFSTRACE, "VFS Op: cfg_instance %s-%016lx (sb %p)\n", profilenm, cfg_instance, sb); - try_module_get(THIS_MODULE); - OBD_ALLOC_PTR(cfg); if (cfg == NULL) GOTO(out_free_cfg, err = -ENOMEM); diff --git a/lustre/llite/super25.c b/lustre/llite/super25.c index 2a510ef..befd246 100644 --- a/lustre/llite/super25.c +++ b/lustre/llite/super25.c @@ -149,8 +149,7 @@ static int __init lustre_init(void) if (rc != 0) GOTO(out_inode_fini_env, rc); - lustre_register_client_fill_super(ll_fill_super); - lustre_register_kill_super_cb(ll_kill_super); + lustre_register_super_ops(THIS_MODULE, ll_fill_super, ll_kill_super); RETURN(0); @@ -169,8 +168,7 @@ out_cache: static void __exit lustre_exit(void) { - lustre_register_client_fill_super(NULL); - lustre_register_kill_super_cb(NULL); + lustre_register_super_ops(NULL, NULL, NULL); llite_tunables_unregister(); diff --git a/lustre/obdclass/obd_mount.c b/lustre/obdclass/obd_mount.c index 538efea..b9c14d2 100644 --- a/lustre/obdclass/obd_mount.c +++ b/lustre/obdclass/obd_mount.c @@ -50,6 +50,8 @@ #include #include +static DEFINE_SPINLOCK(client_lock); +static struct module *client_mod; static int (*client_fill_super)(struct super_block *sb); static void (*kill_super_cb)(struct super_block *sb); @@ -1623,10 +1625,16 @@ static int lustre_fill_super(struct super_block *sb, void *lmd2_data, } if (lmd_is_client(lmd)) { + bool have_client = false; + CDEBUG(D_MOUNT, "Mounting client %s\n", lmd->lmd_profile); - if (client_fill_super == NULL) + if (!client_fill_super) request_module("lustre"); - if (client_fill_super == NULL) { + spin_lock(&client_lock); + if (client_fill_super && try_module_get(client_mod)) + have_client = true; + spin_unlock(&client_lock); + if (!have_client) { LCONSOLE_ERROR_MSG(0x165, "Nothing registered for client mount! Is the 'lustre' module loaded?\n"); lustre_put_lsi(sb); @@ -1640,7 +1648,9 @@ static int lustre_fill_super(struct super_block *sb, void *lmd2_data, /* Connect and start */ /* (should always be ll_fill_super) */ rc = (*client_fill_super)(sb); - /* c_f_s will call lustre_common_put_super on failure */ + /* c_f_s will call lustre_common_put_super on failure, + * which takes care of the module reference. + */ } } else { #ifdef HAVE_SERVER_SUPPORT @@ -1680,17 +1690,17 @@ out: * We can't call ll_fill_super by name because it lives in a module that * must be loaded after this one. */ -void lustre_register_client_fill_super(int (*cfs)(struct super_block *sb)) +void lustre_register_super_ops(struct module *mod, + int (*cfs)(struct super_block *sb), + void (*ksc)(struct super_block *sb)) { + spin_lock(&client_lock); + client_mod = mod; client_fill_super = cfs; + kill_super_cb = ksc; + spin_unlock(&client_lock); } -EXPORT_SYMBOL(lustre_register_client_fill_super); - -void lustre_register_kill_super_cb(void (*cfs)(struct super_block *sb)) -{ - kill_super_cb = cfs; -} -EXPORT_SYMBOL(lustre_register_kill_super_cb); +EXPORT_SYMBOL(lustre_register_super_ops); /***************** FS registration ******************/ #ifdef HAVE_FSTYPE_MOUNT -- 1.8.3.1