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.shevche...@linux.intel.com> Signed-off-by: Reinette Chatre <reinette.cha...@intel.com> --- 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