[PATCH V5 4/4] x86/resctrl: Use appropriate API for strings terminated by newline

From: Reinette Chatre
Date: Mon May 18 2020 - 19:47:24 EST


The user input to files in the resctrl filesystem are expected to be
terminated with a newline. Testing the user input includes a test for
the presence of a newline and then replacing the newline with NUL
byte followed by comparison using strcmp().

sysfs_streq() exists to test if strings are equal, treating both NUL and
newline-then-NUL as equivalent string terminations. Even more,
sysfs_match_string() exists to match a given string in an array using
sysfs_streq().

Replace existing strcmp() comparisons of strings that are terminated
with a newline with more appropriate sysfs_streq() via the
sysfs_match_string() API that can perform the match across the different
mode strings that are already maintained in an array.

Suggested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Signed-off-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
---
Changes since V4:
- Remove "mode" local variable from rdtgroup_mode_write(). This variable
was previously used to create shorter lines with the original strcmp()
code that was removed in patch 4/4.
- Andy pointed out that the repeated assignment to rdtgrp->mode could be
replaced by a single assignment after all the checks. This was initially
rejected because it would let the "RDT_MODE_PSEUDO_LOCKED" assignment
slip through. Even so, Andy's feedback revealed that the new changes
unintentionally let a user's attempt at setting the mode to pseudo-locked
be silently ignored where it previously reported an error. Restore original
user space behavior by returning success when user attempts to change any
mode when it is already the current mode (including pseudo-locked) and
returning failure when user attempts to set the mode to pseudo-locked.
After this change it is possible to follow Andy's original suggestion
of using a single assignment. (Andy)

arch/x86/kernel/cpu/resctrl/rdtgroup.c | 40 ++++++++++++++------------
1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 088a1536bccc..31f1bacaa288 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1414,13 +1414,11 @@ static ssize_t rdtgroup_mode_write(struct kernfs_open_file *of,
char *buf, size_t nbytes, loff_t off)
{
struct rdtgroup *rdtgrp;
- enum rdtgrp_mode mode;
- int ret = 0;
+ int user_m;
+ int ret;

- /* Valid input requires a trailing newline */
- if (nbytes == 0 || buf[nbytes - 1] != '\n')
+ if (nbytes == 0)
return -EINVAL;
- buf[nbytes - 1] = '\0';

rdtgrp = rdtgroup_kn_lock_live(of->kn);
if (!rdtgrp) {
@@ -1430,29 +1428,33 @@ static ssize_t rdtgroup_mode_write(struct kernfs_open_file *of,

rdt_last_cmd_clear();

- mode = rdtgrp->mode;
+ ret = sysfs_match_string(rdt_mode_str, buf);
+ if (ret < 0) {
+ rdt_last_cmd_puts("Unknown or unsupported mode\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ user_m = ret;
+ ret = 0;

- if ((!strcmp(buf, "shareable") && mode == RDT_MODE_SHAREABLE) ||
- (!strcmp(buf, "exclusive") && mode == RDT_MODE_EXCLUSIVE) ||
- (!strcmp(buf, "pseudo-locksetup") &&
- mode == RDT_MODE_PSEUDO_LOCKSETUP) ||
- (!strcmp(buf, "pseudo-locked") && mode == RDT_MODE_PSEUDO_LOCKED))
+ /* Do nothing and return success if user asks for current mode */
+ if (user_m == rdtgrp->mode)
goto out;

- if (mode == RDT_MODE_PSEUDO_LOCKED) {
+ if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED) {
rdt_last_cmd_puts("Cannot change pseudo-locked group\n");
ret = -EINVAL;
goto out;
}

- if (!strcmp(buf, "shareable")) {
+ if (user_m == RDT_MODE_SHAREABLE) {
if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) {
ret = rdtgroup_locksetup_exit(rdtgrp);
if (ret)
goto out;
}
- rdtgrp->mode = RDT_MODE_SHAREABLE;
- } else if (!strcmp(buf, "exclusive")) {
+ } else if (user_m == RDT_MODE_EXCLUSIVE) {
if (!rdtgroup_mode_test_exclusive(rdtgrp)) {
ret = -EINVAL;
goto out;
@@ -1462,16 +1464,16 @@ static ssize_t rdtgroup_mode_write(struct kernfs_open_file *of,
if (ret)
goto out;
}
- rdtgrp->mode = RDT_MODE_EXCLUSIVE;
- } else if (!strcmp(buf, "pseudo-locksetup")) {
+ } else if (user_m == RDT_MODE_PSEUDO_LOCKSETUP) {
ret = rdtgroup_locksetup_enter(rdtgrp);
if (ret)
goto out;
- rdtgrp->mode = RDT_MODE_PSEUDO_LOCKSETUP;
- } else {
+ } else if (user_m == RDT_MODE_PSEUDO_LOCKED) {
rdt_last_cmd_puts("Unknown or unsupported mode\n");
ret = -EINVAL;
+ goto out;
}
+ rdtgrp->mode = user_m;

out:
rdtgroup_kn_unlock(of->kn);
--
2.21.0