Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Likhitha Korrapati <likhitha@linux.ibm.com>
To:     shuah@kernel.org, trenn@suse.com
Cc:     linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        ricklind@linux.vnet.ibm.com, latha@linux.vnet.ibm.com,
        srikar@linux.vnet.ibm.com,
        Likhitha Korrapati <likhitha@linux.ibm.com>,
        Pavithra Prakash <pavrampu@linux.vnet.ibm.com>
Subject: [PATCH v3] cpupower: Fix cpuidle_set to accept only numeric values for idle-set operation.
Date:   Fri, 14 Jul 2023 15:10:21 +0530
Message-Id: <20230714094021.199611-1-likhitha@linux.ibm.com>
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
Precedence: bulk

For both the d and e options in 'cpupower idle_set' command, an
atoi() conversion is done without checking if the input argument
is all numeric. So, an atoi conversion is done on any character
provided as input and the CPU idle_set operation continues with
that integer value, which may not be what is intended or entirely
correct.

The output of cpuidle-set before patch is as follows:

[root@xxx cpupower]# cpupower idle-set -e 1$
Idlestate 1 enabled on CPU 0
[snip]
Idlestate 1 enabled on CPU 47

[root@xxx cpupower]# cpupower idle-set -e 11
Idlestate 11 not available on CPU 0
[snip]
Idlestate 11 not available on CPU 47

[root@xxx cpupower]# cpupower idle-set -d 12
Idlestate 12 not available on CPU 0
[snip]
Idlestate 12 not available on CPU 47

[root@xxx cpupower]# cpupower idle-set -d qw
Idlestate 0 disabled on CPU 0
[snip]
Idlestate 0 disabled on CPU 47

This patch adds a check for both d and e options in cpuidle-set.c
to see that the idle_set value is all numeric before doing a
string-to-int conversion using strtol().

The output of cpuidle-set after the patch is as below:

[root@xxx cpupower]# ./cpupower idle-set -e 1$
Bad idle_set value: 1$. Integer expected

[root@xxx cpupower]# ./cpupower idle-set -e 11
Idlestate 11 not available on CPU 0
[snip]
Idlestate 11 not available on CPU 47

[root@xxx cpupower]# ./cpupower idle-set -d 12
Idlestate 12 not available on CPU 0
[snip]
Idlestate 12 not available on CPU 47

[root@xxx cpupower]# ./cpupower idle-set -d qw
Bad idle_set value: qw. Integer expected

Signed-off-by: Brahadambal Srinivasan <latha@linux.vnet.ibm.com>
Signed-off-by: Likhitha Korrapati <likhitha@linux.ibm.com>
Tested-by: Pavithra Prakash <pavrampu@linux.vnet.ibm.com>
Reviewed-by: Rick Lindsley <ricklind@linux.vnet.ibm.com>
---

** changes since v1 [1] **

- Addressed reviewed comments from v1.
- Slightly reworded the commit for clarity.

** changes since v2 [2] **

- Addressed reviewed comments from v2.

[1] https://lore.kernel.org/all/20210105122452.8687-1-latha@linux.vnet.ibm.com/
[2] https://lore.kernel.org/all/20230410121054.61622-1-likhitha@linux.ibm.com/

 tools/power/cpupower/utils/cpuidle-set.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/tools/power/cpupower/utils/cpuidle-set.c b/tools/power/cpupower/utils/cpuidle-set.c
index 46158928f9ad..a551d1d4ac51 100644
--- a/tools/power/cpupower/utils/cpuidle-set.c
+++ b/tools/power/cpupower/utils/cpuidle-set.c
@@ -41,14 +41,6 @@ int cmd_idle_set(int argc, char **argv)
 			cont = 0;
 			break;
 		case 'd':
-			if (param) {
-				param = -1;
-				cont = 0;
-				break;
-			}
-			param = ret;
-			idlestate = atoi(optarg);
-			break;
 		case 'e':
 			if (param) {
 				param = -1;
@@ -56,7 +48,13 @@ int cmd_idle_set(int argc, char **argv)
 				break;
 			}
 			param = ret;
-			idlestate = atoi(optarg);
+			strtol(optarg, &endptr, 10);
+			if (*endptr != '\0') {
+				printf(_("Bad value: %s, Integer expected\n"), optarg);
+				exit(EXIT_FAILURE);
+			} else {
+				idlestate = atoi(optarg);
+			}
 			break;
 		case 'D':
 			if (param) {
-- 
2.31.1