[PATCH 1/2] PM / devfreq: Fix governor module load failure

From: Ezequiel Garcia <ezequiel@collabora.com>
To: Kyungmin Park <kyungmin.park@samsung.com>,
	MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: kernel@collabora.com, linux-pm@vger.kernel.org,
	Enric Balletbo i Serra <enric.balletbo@collabora.com>,
	Ezequiel Garcia <ezequiel@collabora.com>
Subject: [PATCH 1/2] PM / devfreq: Fix governor module load failure
Date: Wed,  5 Jun 2019 16:00:52 -0300	[thread overview]
Message-ID: <20190605190053.19177-1-ezequiel@collabora.com> (raw)

A bit unexpectedly (but still documented), request_module may
return a positive value, in case of a modprobe error.
This is currently causing issues in the devfreq framework.

When a request_module exits with a positive value, we currently
return that via ERR_PTR. However, because the value is positive,
it's not a ERR_VALUE proper, and is therefore treated as a
valid struct devfreq_governor pointer, leading to a kernel oops.

The right way to fix this is hinted in __request_module documentation:

"""
[snip] The function returns
zero on success or a negative errno code or positive exit code from
"modprobe" on failure. Note that a successful module load does not mean
the module did not then unload and exit on an error of its own. Callers
must check that the service they requested is now available not blindly
invoke it.
"""

Therefore, drop the return value check, which is not useful, and instead
just re-try to find the (hopefully now loaded) governor.

Fixes: 23c7b54ca1cd1 ("PM / devfreq: Fix devfreq_add_device() when drivers are built as modules.")
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/devfreq/devfreq.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 6b6991f0e873..8868ad9472d2 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -236,7 +236,6 @@ static struct devfreq_governor *find_devfreq_governor(const char *name)
 static struct devfreq_governor *try_then_request_governor(const char *name)
 {
 	struct devfreq_governor *governor;
-	int err = 0;
 
 	if (IS_ERR_OR_NULL(name)) {
 		pr_err("DEVFREQ: %s: Invalid parameters\n", __func__);
@@ -251,13 +250,10 @@ static struct devfreq_governor *try_then_request_governor(const char *name)
 
 		if (!strncmp(name, DEVFREQ_GOV_SIMPLE_ONDEMAND,
 			     DEVFREQ_NAME_LEN))
-			err = request_module("governor_%s", "simpleondemand");
+			request_module("governor_%s", "simpleondemand");
 		else
-			err = request_module("governor_%s", name);
-		/* Restore previous state before return */
+			request_module("governor_%s", name);
 		mutex_lock(&devfreq_list_lock);
-		if (err)
-			return ERR_PTR(err);
 
 		governor = find_devfreq_governor(name);
 	}
-- 
2.20.1

