netdev - Re: [PATCH] [v2] devlink: move DEVLINK_ATTR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250709174547.3604c42b@kernel.org>
Date: Wed, 9 Jul 2025 17:45:47 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Arnd Bergmann <arnd@...nel.org>, Carolina Jubran <cjubran@...dia.com>
Cc: Jiri Pirko <jiri@...nulli.us>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Mark
 Bloch <mbloch@...dia.com>, Tariq Toukan <tariqt@...dia.com>, Arnd Bergmann
 <arnd@...db.de>, Simon Horman <horms@...nel.org>, Cosmin Ratiu
 <cratiu@...dia.com>, Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [v2] devlink: move DEVLINK_ATTR_MAX-sized array off
 stack

On Wed,  9 Jul 2025 16:59:00 +0200 Arnd Bergmann wrote:
> -	struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
> +	struct nlattr **tb __free(kfree) = NULL;

Ugh, now you triggered me.

>  	u8 tc_index;
>  	int err;
>  
> +	tb = kcalloc(DEVLINK_ATTR_MAX + 1, sizeof(struct nlattr *), GFP_KERNEL);
> +	if (!tb)
> +		return -ENOMEM;

Cramming all the attributes in a single space is silly, it's better for
devlink to grow up :/ Carolina could you test this?

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 1c4bb0cbe5f0..3d75bc530b30 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -853,18 +853,6 @@ doc: Partial family for Devlink.
         type: nest
         multi-attr: true
         nested-attributes: dl-rate-tc-bws
-      -
-        name: rate-tc-index
-        type: u8
-        checks:
-          max: rate-tc-index-max
-      -
-        name: rate-tc-bw
-        type: u32
-        doc: |
-             Specifies the bandwidth share assigned to the Traffic Class.
-             The bandwidth for the traffic class is determined
-             in proportion to the sum of the shares of all configured classes.
   -
     name: dl-dev-stats
     subset-of: devlink
@@ -1271,12 +1259,20 @@ doc: Partial family for Devlink.
         type: flag
   -
     name: dl-rate-tc-bws
-    subset-of: devlink
+    name-prefix: devlink-attr-
     attributes:
       -
         name: rate-tc-index
+        type: u8
+        checks:
+          max: rate-tc-index-max
       -
         name: rate-tc-bw
+        type: u32
+        doc: |
+             Specifies the bandwidth share assigned to the Traffic Class.
+             The bandwidth for the traffic class is determined
+             in proportion to the sum of the shares of all configured classes.
 
 operations:
   enum-model: directional
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index e72bcc239afd..169a07499556 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -635,8 +635,6 @@ enum devlink_attr {
 	DEVLINK_ATTR_REGION_DIRECT,		/* flag */
 
 	DEVLINK_ATTR_RATE_TC_BWS,		/* nested */
-	DEVLINK_ATTR_RATE_TC_INDEX,		/* u8 */
-	DEVLINK_ATTR_RATE_TC_BW,		/* u32 */
 
 	/* Add new attributes above here, update the spec in
 	 * Documentation/netlink/specs/devlink.yaml and re-generate
@@ -647,6 +645,14 @@ enum devlink_attr {
 	DEVLINK_ATTR_MAX = __DEVLINK_ATTR_MAX - 1
 };
 
+enum {
+	DEVLINK_ATTR_RATE_TC_INDEX = 1,		/* u8 */
+	DEVLINK_ATTR_RATE_TC_BW,		/* u32 */
+
+	__DEVLINK_ATTR_RATE_TC_MAX,
+	DEVLINK_ATTR_RATE_TC_MAX = __DEVLINK_ATTR_RATE_TC_MAX - 1
+};
+
 /* Mapping between internal resource described by the field and system
  * structure
  */
diff --git a/net/devlink/rate.c b/net/devlink/rate.c
index d39300a9b3d4..83ca62ce6c63 100644
--- a/net/devlink/rate.c
+++ b/net/devlink/rate.c
@@ -346,11 +346,11 @@ static int devlink_nl_rate_tc_bw_parse(struct nlattr *parent_nest, u32 *tc_bw,
 				       unsigned long *bitmap,
 				       struct netlink_ext_ack *extack)
 {
-	struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
+	struct nlattr *tb[DEVLINK_ATTR_RATE_TC_MAX + 1];
 	u8 tc_index;
 	int err;
 
-	err = nla_parse_nested(tb, DEVLINK_ATTR_MAX, parent_nest,
+	err = nla_parse_nested(tb, DEVLINK_ATTR_RATE_TC_MAX, parent_nest,
 			       devlink_dl_rate_tc_bws_nl_policy, extack);
 	if (err)
 		return err;