The kmod dups code uses RCU to ensure that a kmod_dup_req instance is freed
only after it is no longer referenced. When releasing an instance, the
kmod_dup_request_delete() function removes the kmod_dup_req from the
dup_kmod_reqs list, waits via synchronize_rcu() and finally frees it.
However, this doesn't work correctly because parallel users referencing the
instance in kmod_dup_request_exists_wait() don't enter an RCU read-side
critical section. This can result in a use-after-free.
The kmod_dup_request_exists_wait() function may need to hold a valid
reference to a kmod_dup_req instance across a blocking wait until the
corresponding modprobe command completes. This makes it unsuitable for RCU.
Fix the issue by changing the lifecycle management of kmod_dup_req to use
reference counting.
Fixes: 8660484ed1cf ("module: add debugging auto-load duplicate module support")
Signed-off-by: Petr Pavlu <[email protected]>
---
kernel/module/dups.c | 43 ++++++++++++++++++++++++++++++++-----------
1 file changed, 32 insertions(+), 11 deletions(-)
diff --git a/kernel/module/dups.c b/kernel/module/dups.c
index 1d720a5311ba..e1828e865edd 100644
--- a/kernel/module/dups.c
+++ b/kernel/module/dups.c
@@ -30,6 +30,7 @@
#include <linux/ptrace.h>
#include <linux/async.h>
#include <linux/uaccess.h>
+#include <linux/refcount.h>
#include "internal.h"
@@ -38,13 +39,12 @@
static bool enable_dups_trace =
IS_ENABLED(CONFIG_MODULE_DEBUG_AUTOLOAD_DUPS_TRACE);
module_param(enable_dups_trace, bool_enable_only, 0644);
-/*
- * Protects dup_kmod_reqs list, adds / removals with RCU.
- */
+/* A mutex-protected list of active kmod requests. */
static DEFINE_MUTEX(kmod_dup_mutex);
static LIST_HEAD(dup_kmod_reqs);
struct kmod_dup_req {
+ refcount_t refcount;
struct list_head list;
char name[MODULE_NAME_LEN];
struct completion first_req_done;
@@ -53,12 +53,24 @@ struct kmod_dup_req {
int dup_ret;
};
+static void get_kmod_req(struct kmod_dup_req *kmod_req)
+{
+ refcount_inc(&kmod_req->refcount);
+}
+
+static void put_kmod_req(struct kmod_dup_req *kmod_req)
+{
+ if (refcount_dec_and_test(&kmod_req->refcount))
+ kfree(kmod_req);
+}
+
static struct kmod_dup_req *kmod_dup_request_lookup(char *module_name)
{
struct kmod_dup_req *kmod_req;
- list_for_each_entry_rcu(kmod_req, &dup_kmod_reqs, list,
- lockdep_is_held(&kmod_dup_mutex)) {
+ lockdep_assert_held(&kmod_dup_mutex);
+
+ list_for_each_entry(kmod_req, &dup_kmod_reqs, list) {
if (strlen(kmod_req->name) == strlen(module_name) &&
!memcmp(kmod_req->name, module_name, strlen(module_name))) {
return kmod_req;
@@ -87,10 +99,10 @@ static void kmod_dup_request_delete(struct work_struct
*work)
* just returning 0.
*/
mutex_lock(&kmod_dup_mutex);
- list_del_rcu(&kmod_req->list);
- synchronize_rcu();
+ list_del(&kmod_req->list);
mutex_unlock(&kmod_dup_mutex);
- kfree(kmod_req);
+
+ put_kmod_req(kmod_req);
}
static void kmod_dup_request_complete(struct work_struct *work)
@@ -129,6 +141,7 @@ bool kmod_dup_request_exists_wait(char *module_name, bool
wait, int *dup_ret)
if (!new_kmod_req)
return false;
+ refcount_set(&new_kmod_req->refcount, 1);
memcpy(new_kmod_req->name, module_name, strlen(module_name));
INIT_WORK(&new_kmod_req->complete_work, kmod_dup_request_complete);
INIT_DELAYED_WORK(&new_kmod_req->delete_work, kmod_dup_request_delete);
@@ -161,10 +174,12 @@ bool kmod_dup_request_exists_wait(char *module_name, bool
wait, int *dup_ret)
* keep tab on duplicates later.
*/
pr_debug("New request_module() for %s\n", module_name);
- list_add_rcu(&new_kmod_req->list, &dup_kmod_reqs);
+ list_add(&new_kmod_req->list, &dup_kmod_reqs);
mutex_unlock(&kmod_dup_mutex);
return false;
}
+
+ get_kmod_req(kmod_req);
mutex_unlock(&kmod_dup_mutex);
/* We are dealing with a duplicate request now */
@@ -194,7 +209,7 @@ bool kmod_dup_request_exists_wait(char *module_name, bool
wait, int *dup_ret)
* calls bail out right away.
*/
*dup_ret = 0;
- return true;
+ goto out;
}
/*
@@ -209,12 +224,14 @@ bool kmod_dup_request_exists_wait(char *module_name, bool
wait, int *dup_ret)
TASK_KILLABLE);
if (ret) {
*dup_ret = ret;
- return true;
+ goto out;
}
/* Now the duplicate request has the same exact return value as the
first request */
*dup_ret = kmod_req->dup_ret;
+out:
+ put_kmod_req(kmod_req);
return true;
}
@@ -224,6 +241,10 @@ void kmod_dup_request_announce(char *module_name, int ret)
mutex_lock(&kmod_dup_mutex);
+ /*
+ * Find the entry previously added in kmod_dup_request_exists_wait()
+ * that is owned by the current task.
+ */
kmod_req = kmod_dup_request_lookup(module_name);
if (!kmod_req)
goto out;
--
2.54.0
