Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 10 May 2019 14:42:04 -0400
In-Reply-To: <be47ac01-a5ac-7be1-d387-5c841007b45f@google.com>
Message-Id: <20190510184204.225451-1-brho@google.com>
Mime-Version: 1.0
References: <be47ac01-a5ac-7be1-d387-5c841007b45f@google.com>
Subject: [PATCH] modules: fix livelock in add_unformed_module()
From:   Barret Rhoden <brho@google.com>
To:     Jessica Yu <jeyu@kernel.org>, Prarit Bhargava <prarit@redhat.com>
Cc:     linux-kernel@vger.kernel.org,
        Heiko Carstens <heiko.carstens@de.ibm.com>,
        David Arcari <darcari@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

When add_unformed_module() finds an existing module with the same name,
it waits until the preexisting module finished loading.  Prior to commit
f9a75c1d717f, this meant if the module was either UNFORMED or COMING,
we'd wait.  That commit changed the check to be !LIVE, so that we'd wait
for UNFORMED, COMING, or GOING.

The problem with that commit was that we wait on finished_loading(), and
that function's state checks were not changed.  If a module was
GOING, finished_loading() was returning true, meaning to recheck the
state and presumably break out of the loop.  This mismatch between the
state checked by add_unformed_module() and the state checked in
finished_loading() could result in a hang.

Specifically, when a module was GOING, wait_event_interruptible() would
immediately return with no error, we'd goto 'again,' see the state !=
LIVE, and try again.

This commit changes finished_loading() such that we only consider a
module 'finished' when it doesn't exist or is LIVE, which are the cases
that break from the wait-loop in add_unformed_module().

Fixes: f9a75c1d717f ("modules: Only return -EEXIST for modules that have finished loading")
Signed-off-by: Barret Rhoden <brho@google.com>
---
 kernel/module.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 410eeb7e4f1d..0744eea7bb90 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3407,8 +3407,7 @@ static bool finished_loading(const char *name)
 	sched_annotate_sleep();
 	mutex_lock(&module_mutex);
 	mod = find_module_all(name, strlen(name), true);
-	ret = !mod || mod->state == MODULE_STATE_LIVE
-		|| mod->state == MODULE_STATE_GOING;
+	ret = !mod || mod->state == MODULE_STATE_LIVE;
 	mutex_unlock(&module_mutex);
 
 	return ret;
-- 
2.21.0.1020.gf2820cf01a-goog