[PATCH] cgroup/rdma: fix strncmp prefix match in parse_resource()

[PATCH] cgroup/rdma: fix strncmp prefix match in parse_resource()

[cuitao]

parse_resource() used strncmp(value, "max", strlen(value)) to match the
"max" keyword. Since strlen(value) is the comparison length, prefixes
like "ma" or "m" are incorrectly accepted as "max".

Fix by replacing strncmp with strcmp for exact matching and remove the
now unused `len` variable.

Signed-off-by: cuitao <cuitao@kylinos.cn>
---
 kernel/cgroup/rdma.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/kernel/cgroup/rdma.c b/kernel/cgroup/rdma.c
index 09258eebb5c7..81e278348dad 100644
--- a/kernel/cgroup/rdma.c
+++ b/kernel/cgroup/rdma.c
@@ -359,7 +359,6 @@ static int parse_resource(char *c, int *intval)
 {
 	substring_t argstr;
 	char *name, *value = c;
-	size_t len;
 	int ret, i;
 
 	name = strsep(&value, "=");
@@ -370,10 +369,8 @@ static int parse_resource(char *c, int *intval)
 	if (i < 0)
 		return i;
 
-	len = strlen(value);
-
 	argstr.from = value;
-	argstr.to = value + len;
+	argstr.to = value + strlen(value);
 
 	ret = match_int(&argstr, intval);
 	if (ret >= 0) {
@@ -381,7 +378,7 @@ static int parse_resource(char *c, int *intval)
 			return -EINVAL;
 		return i;
 	}
-	if (strncmp(value, RDMACG_MAX_STR, len) == 0) {
+	if (strcmp(value, RDMACG_MAX_STR) == 0) {
 		*intval = S32_MAX;
 		return i;
 	}
-- 
2.43.0

Re: [PATCH] cgroup/rdma: fix strncmp prefix match in parse_resource()

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 515 bytes --]

Hello.

On Tue, Apr 14, 2026 at 10:09:36AM +0800, cuitao <cuitao@kylinos.cn> wrote:
>  	}
> -	if (strncmp(value, RDMACG_MAX_STR, len) == 0) {
> +	if (strcmp(value, RDMACG_MAX_STR) == 0) {

Have you tested this? (When 'max' isn't the last assignment.)

That value/c string is taken out of the whole line (see
rdmacg_parse_limits), so it wouldn't be necessarily equal to
RDMACG_MAX_STR. So bounded compare is still somewhat needed:

	if (strncmp(value, RDMACG_MAX_STR, strlen(RDMACG_MAX_STR)) == 0) {

Thanks,
Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]

Re: [PATCH] cgroup/rdma: fix strncmp prefix match in parse_resource()

[崔涛]

在 2026/4/15 21:37, Michal Koutný 写道:
> Hello.
> 
> On Tue, Apr 14, 2026 at 10:09:36AM +0800, cuitao <cuitao@kylinos.cn> wrote:
>>  	}
>> -	if (strncmp(value, RDMACG_MAX_STR, len) == 0) {
>> +	if (strcmp(value, RDMACG_MAX_STR) == 0) {
> 
> Have you tested this? (When 'max' isn't the last assignment.)
Yes, I have tested this. Thank you for raising a valid concern about the parsing context. However, after investigation and testing, I believe strcmp is still the correct approach. Let me explain with test results from three kernel versions:

Test 1: Original code（mainline) — strncmp(value, RDMACG_MAX_STR, strlen(value))

echo "... hca_handle=ma hca_object=20"	 → accepted (BUG: "ma" matches "max")
echo "... hca_handle=maxaa hca_object=20" → rejected
echo "... hca_handle=max     hca_object=20" → rejected

Test 2: strcmp(value, RDMACG_MAX_STR) — this commit

echo "... hca_handle=ma hca_object=20" 	→ rejected
echo "... hca_handle=maxaa hca_object=20" → rejected
echo "... hca_handle=max     hca_object=20" → rejected

Test 3: strncmp(value, RDMACG_MAX_STR, strlen(RDMACG_MAX_STR)) — your suggestion

echo "... hca_handle=ma hca_object=20" 	→ rejected
echo "... hca_handle=maxaa hca_object=20" → accepted (BUG: "maxaa" matches "max")
echo "... hca_handle=max     hca_object=20" → rejected

The suggested strncmp approach actually introduces a new bug: it would accept "maxaa" as "max" because it only compares the first strlen("max") = 3 characters.
> 
> That value/c string is taken out of the whole line (see
> rdmacg_parse_limits), so it wouldn't be necessarily equal to
> RDMACG_MAX_STR. So bounded compare is still somewhat needed:
> 
> 	if (strncmp(value, RDMACG_MAX_STR, strlen(RDMACG_MAX_STR)) == 0) {
> 
Regarding the concern that value might not be exactly "max" when it's not the last assignment — the "max + extra spaces" test above demonstrates that rdmacg_parse_limits() uses strsep(&options, " ") to split the input by spaces, so by the time parse_resource() receives the value, it is already a clean NUL-terminated token. The extra spaces create empty sub-tokens that fail earlier in validation, regardless of which comparison method is used. So the value passed to the max-check is always either exactly "max", a number, or an invalid string — never "max" with trailing content from a subsequent assignment.

Therefore, an exact string match with strcmp is both correct and sufficient.

[  383.716179] rdmacg: set_max: raw input='rocep1s0f0 hca_handle=max   hca_object=20'
[  383.716184] rdmacg: set_max: dev_name='rocep1s0f0', options='hca_handle=max   hca_object=20'
[  383.716185] rdmacg: parse_limits: token='hca_handle=max'
[  383.716188] rdmacg: parse_resource: 'hca_handle'=max (ok)
[  383.716189] rdmacg: parse_limits: token=''
[  383.716189] rdmacg: parse_resource: missing name or value (input='')
[  383.716198] rdmacg: parse_limits: failed on token '' (err=-22)
[  383.716199] rdmacg: parse_limits: failed (err=-22)
[  383.716200] rdmacg: set_max: parse_limits failed (err=-22)
[  499.213562] rdmacg: set_max: raw input='rocep1s0f0 hca_handle=max hca_object=20'
[  499.213566] rdmacg: set_max: dev_name='rocep1s0f0', options='hca_handle=max hca_object=20'
[  499.213568] rdmacg: parse_limits: token='hca_handle=max'
[  499.213570] rdmacg: parse_resource: 'hca_handle'=max (ok)
[  499.213571] rdmacg: parse_limits: token='hca_object=20'
[  499.213572] rdmacg: parse_resource: 'hca_object'=20 (ok)
[  499.213574] rdmacg: parse_limits: all tokens parsed (ok)
[  499.213575] rdmacg: set_max: device 'rocep1s0f0' limits updated (ok)

> Thanks,
> Michal
Thanks,
cuitao

Re: [PATCH] cgroup/rdma: fix strncmp prefix match in parse_resource()

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 947 bytes --]

On Fri, Apr 17, 2026 at 02:43:43PM +0800, 崔涛 <cuitao@kylinos.cn> wrote:
> Test 3: strncmp(value, RDMACG_MAX_STR, strlen(RDMACG_MAX_STR)) — your suggestion
> 
> echo "... hca_handle=ma hca_object=20" 	→ rejected
> echo "... hca_handle=maxaa hca_object=20" → accepted (BUG: "maxaa" matches "max")
> echo "... hca_handle=max     hca_object=20" → rejected
> 
> The suggested strncmp approach actually introduces a new bug: it would
> accept "maxaa" as "max" because it only compares the first
> strlen("max") = 3 characters.

True, I missed this.

> The extra spaces create empty sub-tokens that fail earlier in
> validation, regardless of which comparison method is used.

Yes, this is suckage too (also in your Test 2 third result).

As I look around (tg_set_limit, user_proactive_reclaim,
ioc_cost_model_write), this would be most cleanly tackled with a
match_table_t and match_token().

WDYT?

Thanks,
Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]

Re: [PATCH] cgroup/rdma: fix strncmp prefix match in parse_resource()

[Tao Cui]

在 2026/4/17 17:53, Michal Koutný 写道:
> On Fri, Apr 17, 2026 at 02:43:43PM +0800, 崔涛 <cuitao@kylinos.cn> wrote:
>> Test 3: strncmp(value, RDMACG_MAX_STR, strlen(RDMACG_MAX_STR)) — your suggestion
>>
>> echo "... hca_handle=ma hca_object=20" 	→ rejected
>> echo "... hca_handle=maxaa hca_object=20" → accepted (BUG: "maxaa" matches "max")
>> echo "... hca_handle=max     hca_object=20" → rejected
>>
>> The suggested strncmp approach actually introduces a new bug: it would
>> accept "maxaa" as "max" because it only compares the first
>> strlen("max") = 3 characters.
> 
> True, I missed this.
> 
>> The extra spaces create empty sub-tokens that fail earlier in
>> validation, regardless of which comparison method is used.
> 
> Yes, this is suckage too (also in your Test 2 third result).
> 
> As I look around (tg_set_limit, user_proactive_reclaim,
> ioc_cost_model_write), this would be most cleanly tackled with a
> match_table_t and match_token().
> 
> WDYT?
> 
I’ve also gone through the relevant code of tg_set_limit, user_proactive_reclaim and ioc_cost_model_write. Let me have a go at implementing it this way.

Thanks,
cuitao
> Thanks,
> Michal

[PATCH v2] cgroup/rdma: refactor resource parsing with match_table_t/match_token()

[cuitao]

Replace the hand-rolled strsep/strcmp/match_string parsing in
rdmacg_resource_set_max() with a match_table_t and match_token()
pattern, following the convention used by user_proactive_reclaim()
and ioc_cost_model_write().

This fixes the original strncmp(value, RDMACG_MAX_STR, strlen(value))
bug that matched "ma" as "max", and also robustly handles extra
whitespace in the input by splitting on " \t\n" and skipping empty
tokens.

Suggested-by: "Michal Koutný" <mkoutny@suse.com>
Signed-off-by: cuitao <cuitao@kylinos.cn>

---
Changes in v2:
- Refactor to use match_table_t/match_token() as suggested by Michal Koutný
  Link: https://lore.kernel.org/all/t2cbjvctdgzipxzovr5zkbovhptkxdaoxljeuxwrxboqqbkzqu@bcazimapp6ci/
---
 kernel/cgroup/rdma.c | 120 ++++++++++++++++++++-----------------------
 1 file changed, 57 insertions(+), 63 deletions(-)

diff --git a/kernel/cgroup/rdma.c b/kernel/cgroup/rdma.c
index 9967fb25c563..48d6bc7d9624 100644
--- a/kernel/cgroup/rdma.c
+++ b/kernel/cgroup/rdma.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/limits.h>
 #include <linux/slab.h>
 #include <linux/seq_file.h>
 #include <linux/cgroup.h>
@@ -17,6 +18,22 @@
 
 #define RDMACG_MAX_STR "max"
 
+enum rdmacg_limit_tokens {
+	RDMACG_HCA_HANDLE_VAL,
+	RDMACG_HCA_HANDLE_MAX,
+	RDMACG_HCA_OBJECT_VAL,
+	RDMACG_HCA_OBJECT_MAX,
+	NR_RDMACG_LIMIT_TOKENS,
+};
+
+static const match_table_t rdmacg_limit_tokens = {
+	{ RDMACG_HCA_HANDLE_VAL,	"hca_handle=%d"	},
+	{ RDMACG_HCA_HANDLE_MAX,	"hca_handle=max"	},
+	{ RDMACG_HCA_OBJECT_VAL,	"hca_object=%d"	},
+	{ RDMACG_HCA_OBJECT_MAX,	"hca_object=max"	},
+	{ NR_RDMACG_LIMIT_TOKENS,	NULL			},
+};
+
 /*
  * Protects list of resource pools maintained on per cgroup basis
  * and rdma device list.
@@ -355,62 +372,6 @@ void rdmacg_unregister_device(struct rdmacg_device *device)
 }
 EXPORT_SYMBOL(rdmacg_unregister_device);
 
-static int parse_resource(char *c, int *intval)
-{
-	substring_t argstr;
-	char *name, *value = c;
-	size_t len;
-	int ret, i;
-
-	name = strsep(&value, "=");
-	if (!name || !value)
-		return -EINVAL;
-
-	i = match_string(rdmacg_resource_names, RDMACG_RESOURCE_MAX, name);
-	if (i < 0)
-		return i;
-
-	len = strlen(value);
-
-	argstr.from = value;
-	argstr.to = value + len;
-
-	ret = match_int(&argstr, intval);
-	if (ret >= 0) {
-		if (*intval < 0)
-			return -EINVAL;
-		return i;
-	}
-	if (strncmp(value, RDMACG_MAX_STR, len) == 0) {
-		*intval = S32_MAX;
-		return i;
-	}
-	return -EINVAL;
-}
-
-static int rdmacg_parse_limits(char *options,
-			       int *new_limits, unsigned long *enables)
-{
-	char *c;
-	int err = -EINVAL;
-
-	/* parse resource options */
-	while ((c = strsep(&options, " ")) != NULL) {
-		int index, intval;
-
-		index = parse_resource(c, &intval);
-		if (index < 0)
-			goto err;
-
-		new_limits[index] = intval;
-		*enables |= BIT(index);
-	}
-	return 0;
-
-err:
-	return err;
-}
-
 static struct rdmacg_device *rdmacg_get_device_locked(const char *name)
 {
 	struct rdmacg_device *device;
@@ -432,6 +393,7 @@ static ssize_t rdmacg_resource_set_max(struct kernfs_open_file *of,
 	struct rdmacg_resource_pool *rpool;
 	struct rdmacg_device *device;
 	char *options = strstrip(buf);
+	char *p;
 	int *new_limits;
 	unsigned long enables = 0;
 	int i = 0, ret = 0;
@@ -449,9 +411,45 @@ static ssize_t rdmacg_resource_set_max(struct kernfs_open_file *of,
 		goto err;
 	}
 
-	ret = rdmacg_parse_limits(options, new_limits, &enables);
-	if (ret)
-		goto parse_err;
+	/* parse resource limit tokens */
+	while ((p = strsep(&options, " \t\n"))) {
+		substring_t args[MAX_OPT_ARGS];
+		int tok, intval;
+
+		if (!*p)
+			continue;
+
+		tok = match_token(p, rdmacg_limit_tokens, args);
+		switch (tok) {
+		case RDMACG_HCA_HANDLE_VAL:
+			if (match_int(&args[0], &intval) || intval < 0) {
+				ret = -EINVAL;
+				goto parse_err;
+			}
+			new_limits[RDMACG_RESOURCE_HCA_HANDLE] = intval;
+			enables |= BIT(RDMACG_RESOURCE_HCA_HANDLE);
+			break;
+		case RDMACG_HCA_HANDLE_MAX:
+			new_limits[RDMACG_RESOURCE_HCA_HANDLE] = S32_MAX;
+			enables |= BIT(RDMACG_RESOURCE_HCA_HANDLE);
+			break;
+		case RDMACG_HCA_OBJECT_VAL:
+			if (match_int(&args[0], &intval) || intval < 0) {
+				ret = -EINVAL;
+				goto parse_err;
+			}
+			new_limits[RDMACG_RESOURCE_HCA_OBJECT] = intval;
+			enables |= BIT(RDMACG_RESOURCE_HCA_OBJECT);
+			break;
+		case RDMACG_HCA_OBJECT_MAX:
+			new_limits[RDMACG_RESOURCE_HCA_OBJECT] = S32_MAX;
+			enables |= BIT(RDMACG_RESOURCE_HCA_OBJECT);
+			break;
+		default:
+			ret = -EINVAL;
+			goto parse_err;
+		}
+	}
 
 	/* acquire lock to synchronize with hot plug devices */
 	mutex_lock(&rdmacg_mutex);
@@ -474,10 +472,6 @@ static ssize_t rdmacg_resource_set_max(struct kernfs_open_file *of,
 
 	if (rpool->usage_sum == 0 &&
 	    rpool->num_max_cnt == RDMACG_RESOURCE_MAX) {
-		/*
-		 * No user of the rpool and all entries are set to max, so
-		 * safe to delete this rpool.
-		 */
 		free_cg_rpool_locked(rpool);
 	}
 
-- 
2.43.0

Re: [PATCH v2] cgroup/rdma: refactor resource parsing with match_table_t/match_token()

[Tejun Heo]

Hello,

A few things for a respin:

- Can you use your real name in the Signed-off-by? Something like
  "Firstname Lastname <email>" rather than the account handle.

- The patch also drops this comment in rdmacg_resource_set_max():

    /*
     * No user of the rpool and all entries are set to max, so
     * safe to delete this rpool.
     */

  That's unrelated to the parser refactor. Please keep it, or split
  it out as a separate cleanup patch.

- The old strncmp(value, RDMACG_MAX_STR, strlen(value)) also silently
  accepted "hca_handle=" (empty value) as "max" because strncmp with
  n=0 always returns 0. v2 fixes this too - worth mentioning in the
  changelog alongside the "ma" case.

Thanks.

--
tejun