[PATCH net-next 0/4] ynl/ethtool/netlink: warn nla_len overflow for large string sets

[PATCH net-next 0/4] ynl/ethtool/netlink: warn nla_len overflow for large string sets

[Hangbin Liu]

This series addresses a silent data corruption issue triggered when ynl
retrieves string sets from NICs with a large number of statistics entries
(e.g. mlx5_core with thousands of ETH_SS_STATS strings).

The root cause is that struct nlattr.nla_len is a __u16 (max 65535
bytes). When a NIC exports enough statistics strings, the
ETHTOOL_A_STRINGSET_STRINGS nest built by strset_fill_set() exceeds
this limit. nla_nest_end() silently truncates the length on assignment,
producing a corrupted netlink message. In the doit path the corrupted
message is delivered to userspace without any error; in the dumpit path
an -EMSGSIZE may be returned if the data does not fit in the dump skb.

Patch 1 improves the userspace tool: rename the doit/dumpit helpers
to do_set/do_get and convert do_get to use ynl.do() with an
explicit device header instead of a full dump with client-side filtering.

Patch 2 adds a --dbg-small-recv option to the YNL ethtool tool,
matching the same option already present in cli.py, to aid debugging
of netlink message size issues.

Patch 3 is the kernel fix: check whether the strings_attr nest would
exceed U16_MAX before calling nla_nest_end() in strset_fill_set(),
and return -EMSGSIZE early if so.

Patch 4 adds a generic WARN_ON_ONCE() in nla_nest_end() itself, so
that any future caller hitting the same overflow is immediately visible
in the kernel log rather than silently corrupting data.

---
Hangbin Liu (4):
      tools: ynl: ethtool: use doit instead of dumpit for per-device GET
      tools: ynl: ethtool: add --dbg-small-recv option
      ethtool: strset: check nla_len overflow before nla_nest_end
      netlink: warn on nla_len overflow in nla_nest_end()

 include/net/netlink.h          |  1 +
 net/ethtool/strset.c           |  4 +++
 tools/net/ynl/pyynl/ethtool.py | 77 ++++++++++++++++++++++--------------------
 3 files changed, 46 insertions(+), 36 deletions(-)
---
base-commit: dc9e9d61e301c087bcd990dbf2fa18ad3e2e1429
change-id: 20260324-b4-ynl_ethtool-f87cd42f572c

Best regards,
-- 
Hangbin Liu <liuhangbin@gmail.com>

[PATCH net-next 1/4] tools: ynl: ethtool: use doit instead of dumpit for per-device GET

[Hangbin Liu]

Rename the local helper doit() to do_set() and dumpit() to do_get() to
better reflect their purpose.

Convert do_get() to use ynl.do() with an explicit device header instead
of ynl.dump() followed by client-side filtering. This is more efficient
as the kernel only processes and returns data for the requested device,
rather than dumping all devices across the netns.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 tools/net/ynl/pyynl/ethtool.py | 68 ++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 35 deletions(-)

diff --git a/tools/net/ynl/pyynl/ethtool.py b/tools/net/ynl/pyynl/ethtool.py
index f1a2a2a89985..8bf234d594b3 100755
--- a/tools/net/ynl/pyynl/ethtool.py
+++ b/tools/net/ynl/pyynl/ethtool.py
@@ -84,9 +84,9 @@ def print_speed(name, value):
     speed = [ k for k, v in value.items() if v and speed_re.match(k) ]
     print(f'{name}: {" ".join(speed)}')
 
-def doit(ynl, args, op_name):
+def do_set(ynl, args, op_name):
     """
-    Prepare request header, parse arguments and doit.
+    Prepare request header, parse arguments and do a set operation.
     """
     req = {
         'header': {
@@ -97,26 +97,24 @@ def doit(ynl, args, op_name):
     args_to_req(ynl, op_name, args.args, req)
     ynl.do(op_name, req)
 
-def dumpit(ynl, args, op_name, extra=None):
+def do_get(ynl, args, op_name, extra=None):
     """
-    Prepare request header, parse arguments and dumpit (filtering out the
-    devices we're not interested in).
+    Prepare request header and get info for a specific device using doit.
     """
     extra = extra or {}
-    reply = ynl.dump(op_name, { 'header': {} } | extra)
+    req = {'header': {'dev-name': args.device}}
+    req['header'].update(extra.pop('header', {}))
+    req.update(extra)
+
+    reply = ynl.do(op_name, req)
     if not reply:
         return {}
 
-    for msg in reply:
-        if msg['header']['dev-name'] == args.device:
-            if args.json:
-                pprint.PrettyPrinter().pprint(msg)
-                sys.exit(0)
-            msg.pop('header', None)
-            return msg
-
-    print(f"Not supported for device {args.device}")
-    sys.exit(1)
+    if args.json:
+        pprint.PrettyPrinter().pprint(reply)
+        sys.exit(0)
+    reply.pop('header', None)
+    return reply
 
 def bits_to_dict(attr):
     """
@@ -181,15 +179,15 @@ def main():
         return
 
     if args.set_eee:
-        doit(ynl, args, 'eee-set')
+        do_set(ynl, args, 'eee-set')
         return
 
     if args.set_pause:
-        doit(ynl, args, 'pause-set')
+        do_set(ynl, args, 'pause-set')
         return
 
     if args.set_coalesce:
-        doit(ynl, args, 'coalesce-set')
+        do_set(ynl, args, 'coalesce-set')
         return
 
     if args.set_features:
@@ -198,20 +196,20 @@ def main():
         return
 
     if args.set_channels:
-        doit(ynl, args, 'channels-set')
+        do_set(ynl, args, 'channels-set')
         return
 
     if args.set_ring:
-        doit(ynl, args, 'rings-set')
+        do_set(ynl, args, 'rings-set')
         return
 
     if args.show_priv_flags:
-        flags = bits_to_dict(dumpit(ynl, args, 'privflags-get')['flags'])
+        flags = bits_to_dict(do_get(ynl, args, 'privflags-get')['flags'])
         print_field(flags)
         return
 
     if args.show_eee:
-        eee = dumpit(ynl, args, 'eee-get')
+        eee = do_get(ynl, args, 'eee-get')
         ours = bits_to_dict(eee['modes-ours'])
         peer = bits_to_dict(eee['modes-peer'])
 
@@ -232,18 +230,18 @@ def main():
         return
 
     if args.show_pause:
-        print_field(dumpit(ynl, args, 'pause-get'),
+        print_field(do_get(ynl, args, 'pause-get'),
                 ('autoneg', 'Autonegotiate', 'bool'),
                 ('rx', 'RX', 'bool'),
                 ('tx', 'TX', 'bool'))
         return
 
     if args.show_coalesce:
-        print_field(dumpit(ynl, args, 'coalesce-get'))
+        print_field(do_get(ynl, args, 'coalesce-get'))
         return
 
     if args.show_features:
-        reply = dumpit(ynl, args, 'features-get')
+        reply = do_get(ynl, args, 'features-get')
         available = bits_to_dict(reply['hw'])
         requested = bits_to_dict(reply['wanted']).keys()
         active = bits_to_dict(reply['active']).keys()
@@ -270,7 +268,7 @@ def main():
         return
 
     if args.show_channels:
-        reply = dumpit(ynl, args, 'channels-get')
+        reply = do_get(ynl, args, 'channels-get')
         print(f'Channel parameters for {args.device}:')
 
         print('Pre-set maximums:')
@@ -290,7 +288,7 @@ def main():
         return
 
     if args.show_ring:
-        reply = dumpit(ynl, args, 'channels-get')
+        reply = do_get(ynl, args, 'channels-get')
 
         print(f'Ring parameters for {args.device}:')
 
@@ -319,7 +317,7 @@ def main():
         print('NIC statistics:')
 
         # TODO: pass id?
-        strset = dumpit(ynl, args, 'strset-get')
+        strset = do_get(ynl, args, 'strset-get')
         pprint.PrettyPrinter().pprint(strset)
 
         req = {
@@ -338,7 +336,7 @@ def main():
           },
         }
 
-        rsp = dumpit(ynl, args, 'stats-get', req)
+        rsp = do_get(ynl, args, 'stats-get', req)
         pprint.PrettyPrinter().pprint(rsp)
         return
 
@@ -349,7 +347,7 @@ def main():
           },
         }
 
-        tsinfo = dumpit(ynl, args, 'tsinfo-get', req)
+        tsinfo = do_get(ynl, args, 'tsinfo-get', req)
 
         print(f'Time stamping parameters for {args.device}:')
 
@@ -377,7 +375,7 @@ def main():
         return
 
     print(f'Settings for {args.device}:')
-    linkmodes = dumpit(ynl, args, 'linkmodes-get')
+    linkmodes = do_get(ynl, args, 'linkmodes-get')
     ours = bits_to_dict(linkmodes['ours'])
 
     supported_ports = ('TP',  'AUI', 'BNC', 'MII', 'FIBRE', 'Backplane')
@@ -425,7 +423,7 @@ def main():
             5: 'Directly Attached Copper',
             0xef: 'None',
     }
-    linkinfo = dumpit(ynl, args, 'linkinfo-get')
+    linkinfo = do_get(ynl, args, 'linkinfo-get')
     print(f'Port: {ports.get(linkinfo["port"], "Other")}')
 
     print_field(linkinfo, ('phyaddr', 'PHYAD'))
@@ -447,11 +445,11 @@ def main():
         mdix = mdix_ctrl.get(linkinfo['tp-mdix'], 'Unknown (auto)')
     print(f'MDI-X: {mdix}')
 
-    debug = dumpit(ynl, args, 'debug-get')
+    debug = do_get(ynl, args, 'debug-get')
     msgmask = bits_to_dict(debug.get("msgmask", [])).keys()
     print(f'Current message level: {" ".join(msgmask)}')
 
-    linkstate = dumpit(ynl, args, 'linkstate-get')
+    linkstate = do_get(ynl, args, 'linkstate-get')
     detected_states = {
             0: 'no',
             1: 'yes',

-- 
Git-155)

[PATCH net-next 2/4] tools: ynl: ethtool: add --dbg-small-recv option

[Hangbin Liu]

Add a --dbg-small-recv debug option to control the recv() buffer size
used by YNL, matching the same option already present in cli.py. This
is useful if user need to get large netlink message.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 tools/net/ynl/pyynl/ethtool.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/pyynl/ethtool.py b/tools/net/ynl/pyynl/ethtool.py
index 8bf234d594b3..6c50368de967 100755
--- a/tools/net/ynl/pyynl/ethtool.py
+++ b/tools/net/ynl/pyynl/ethtool.py
@@ -166,12 +166,19 @@ def main():
     parser.add_argument('device', metavar='device', type=str)
     parser.add_argument('args', metavar='args', type=str, nargs='*')
 
+    dbg_group = parser.add_argument_group('Debug options')
+    dbg_group.add_argument('--dbg-small-recv', default=0, const=4000,
+                           action='store', nargs='?', type=int, metavar='INT',
+                           help="Length of buffers used for recv()")
+
     args = parser.parse_args()
 
     spec = os.path.join(spec_dir(), 'ethtool.yaml')
     schema = os.path.join(schema_dir(), 'genetlink-legacy.yaml')
 
-    ynl = YnlFamily(spec, schema)
+    ynl = YnlFamily(spec, schema, recv_size=args.dbg_small_recv)
+    if args.dbg_small_recv:
+        ynl.set_recv_dbg(True)
 
     if args.set_priv_flags:
         # TODO: parse the bitmask

-- 
Git-155)

[PATCH net-next 3/4] ethtool: strset: check nla_len overflow before nla_nest_end

[Hangbin Liu]

The netlink attribute length field nla_len is a __u16, which can only
represent values up to 65535 bytes. NICs with a large number of
statistics strings (e.g. mlx5_core with thousands of ETH_SS_STATS
entries) can produce a ETHTOOL_A_STRINGSET_STRINGS nest that exceeds
this limit.

When nla_nest_end() writes the actual nest size back to nla_len, the
value is silently truncated. This results in a corrupted netlink message
being sent to userspace: the parser reads a wrong (truncated) attribute
length and misaligns all subsequent attribute boundaries, causing decode
errors.

Fix this by checking whether the size of strings_attr would exceed
U16_MAX after all strings have been written, and give up nla put if so.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/ethtool/strset.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index f6a67109beda..9c502b290f5c 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -441,6 +441,10 @@ static int strset_fill_set(struct sk_buff *skb,
 			if (strset_fill_string(skb, set_info, i) < 0)
 				goto nla_put_failure;
 		}
+
+		if (skb_tail_pointer(skb) - (unsigned char *)strings_attr > U16_MAX)
+			goto nla_put_failure;
+
 		nla_nest_end(skb, strings_attr);
 	}
 

-- 
Git-155)

[PATCH net-next 4/4] netlink: warn on nla_len overflow in nla_nest_end()

[Hangbin Liu]

The nla_len field in struct nlattr is a __u16, which can only hold
values up to 65535. If a nested attribute grows beyond this limit,
nla_nest_end() silently truncates the length, producing a corrupted
netlink message with no indication of the problem.

Since this is unlikely to happen, to avoid unnecessary checking every
time on the production system, add a DEBUG_NET_WARN_ON_ONCE() before
the assignment to make this overflow visible in the debug kernel log.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 include/net/netlink.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 1a8356ca4b78..00ea52dc08c4 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -2260,6 +2260,7 @@ static inline struct nlattr *nla_nest_start(struct sk_buff *skb, int attrtype)
  */
 static inline int nla_nest_end(struct sk_buff *skb, struct nlattr *start)
 {
+	DEBUG_NET_WARN_ON_ONCE(skb_tail_pointer(skb) - (unsigned char *)start > U16_MAX);
 	start->nla_len = skb_tail_pointer(skb) - (unsigned char *)start;
 	return skb->len;
 }

-- 
Git-155)

Re: [PATCH net-next 3/4] ethtool: strset: check nla_len overflow before nla_nest_end

[Jakub Kicinski]

On Tue, 31 Mar 2026 11:56:13 +0800 Hangbin Liu wrote:
> +		if (skb_tail_pointer(skb) - (unsigned char *)strings_attr > U16_MAX)
> +			goto nla_put_failure;

bit ugly, let's add a variant of nla_nest_end() which can return 
an error on overflow (without the warning from patch 4) ?

> +
>  		nla_nest_end(skb, strings_attr);

Re: [PATCH net-next 1/4] tools: ynl: ethtool: use doit instead of dumpit for per-device GET

[Jakub Kicinski]

On Tue, 31 Mar 2026 11:56:11 +0800 Hangbin Liu wrote:
> +++ b/tools/net/ynl/pyynl/ethtool.py

We have converted all the samples to selftests so this script is 
the last piece of random "PoC" code we still have laying around.
Should we also move it to tests/ ?

If there's a reason to keep it as is -- could you please add that reason
at the top of the file for my future self? I'm pretty sure I already
questioned this script in the past ;)

Re: [PATCH net-next 1/4] tools: ynl: ethtool: use doit instead of dumpit for per-device GET

[Hangbin Liu]

On Tue, Mar 31, 2026 at 06:50:28PM -0700, Jakub Kicinski wrote:
> On Tue, 31 Mar 2026 11:56:11 +0800 Hangbin Liu wrote:
> > +++ b/tools/net/ynl/pyynl/ethtool.py
> 
> We have converted all the samples to selftests so this script is 
> the last piece of random "PoC" code we still have laying around.
> Should we also move it to tests/ ?
> 
> If there's a reason to keep it as is -- could you please add that reason
> at the top of the file for my future self? I'm pretty sure I already
> questioned this script in the past ;)

I have no objection to this. Will you keep it as it is when moving it to
tests/ ? Should I wait for your moving first?

Hangbin

Re: [PATCH net-next 3/4] ethtool: strset: check nla_len overflow before nla_nest_end

[Hangbin Liu]

On Tue, Mar 31, 2026 at 06:46:37PM -0700, Jakub Kicinski wrote:
> On Tue, 31 Mar 2026 11:56:13 +0800 Hangbin Liu wrote:
> > +		if (skb_tail_pointer(skb) - (unsigned char *)strings_attr > U16_MAX)
> > +			goto nla_put_failure;
> 
> bit ugly, let's add a variant of nla_nest_end() which can return 
> an error on overflow (without the warning from patch 4) ?

I was tried to not touch nla_nest_end() as it is used everywhere. But it makes
sense to me to add a new function to check this. I'm not very good at naming,
maybe `nla_nest_end_validate()` ? Or any other name if you have?

Thanks
Hangbin
> 
> > +
> >  		nla_nest_end(skb, strings_attr);

Re: [PATCH net-next 3/4] ethtool: strset: check nla_len overflow before nla_nest_end

[Jakub Kicinski]

On Wed, 1 Apr 2026 07:27:40 +0000 Hangbin Liu wrote:
> On Tue, Mar 31, 2026 at 06:46:37PM -0700, Jakub Kicinski wrote:
> > On Tue, 31 Mar 2026 11:56:13 +0800 Hangbin Liu wrote:  
> > > +		if (skb_tail_pointer(skb) - (unsigned char *)strings_attr > U16_MAX)
> > > +			goto nla_put_failure;  
> > 
> > bit ugly, let's add a variant of nla_nest_end() which can return 
> > an error on overflow (without the warning from patch 4) ?  
> 
> I was tried to not touch nla_nest_end() as it is used everywhere. But it makes
> sense to me to add a new function to check this. I'm not very good at naming,
> maybe `nla_nest_end_validate()` ? Or any other name if you have?

I was thinking nla_nest_end_check() to match the __must_check
attribute. But I also like nla_nest_end_safe()
_validate() is a little long but anything <=5 chars is fine.

Re: [PATCH net-next 1/4] tools: ynl: ethtool: use doit instead of dumpit for per-device GET

[Jakub Kicinski]

On Wed, 1 Apr 2026 07:21:54 +0000 Hangbin Liu wrote:
> On Tue, Mar 31, 2026 at 06:50:28PM -0700, Jakub Kicinski wrote:
> > On Tue, 31 Mar 2026 11:56:11 +0800 Hangbin Liu wrote:  
> > > +++ b/tools/net/ynl/pyynl/ethtool.py  
> > 
> > We have converted all the samples to selftests so this script is 
> > the last piece of random "PoC" code we still have laying around.
> > Should we also move it to tests/ ?
> > 
> > If there's a reason to keep it as is -- could you please add that reason
> > at the top of the file for my future self? I'm pretty sure I already
> > questioned this script in the past ;)  
> 
> I have no objection to this. Will you keep it as it is when moving it to
> tests/ ? Should I wait for your moving first?

Would you mind moving it as part of your series?

Re: [PATCH net-next 1/4] tools: ynl: ethtool: use doit instead of dumpit for per-device GET

[Hangbin Liu]

On Wed, Apr 01, 2026 at 05:51:17PM -0700, Jakub Kicinski wrote:
> On Wed, 1 Apr 2026 07:21:54 +0000 Hangbin Liu wrote:
> > On Tue, Mar 31, 2026 at 06:50:28PM -0700, Jakub Kicinski wrote:
> > > On Tue, 31 Mar 2026 11:56:11 +0800 Hangbin Liu wrote:  
> > > > +++ b/tools/net/ynl/pyynl/ethtool.py  
> > > 
> > > We have converted all the samples to selftests so this script is 
> > > the last piece of random "PoC" code we still have laying around.
> > > Should we also move it to tests/ ?
> > > 
> > > If there's a reason to keep it as is -- could you please add that reason
> > > at the top of the file for my future self? I'm pretty sure I already
> > > questioned this script in the past ;)  
> > 
> > I have no objection to this. Will you keep it as it is when moving it to
> > tests/ ? Should I wait for your moving first?
> 
> Would you mind moving it as part of your series?

OK, I will do it.

Thanks
Hangbin

Re: [PATCH net-next 3/4] ethtool: strset: check nla_len overflow before nla_nest_end

[Hangbin Liu]

On Wed, Apr 01, 2026 at 05:49:56PM -0700, Jakub Kicinski wrote:
> On Wed, 1 Apr 2026 07:27:40 +0000 Hangbin Liu wrote:
> > On Tue, Mar 31, 2026 at 06:46:37PM -0700, Jakub Kicinski wrote:
> > > On Tue, 31 Mar 2026 11:56:13 +0800 Hangbin Liu wrote:  
> > > > +		if (skb_tail_pointer(skb) - (unsigned char *)strings_attr > U16_MAX)
> > > > +			goto nla_put_failure;  
> > > 
> > > bit ugly, let's add a variant of nla_nest_end() which can return 
> > > an error on overflow (without the warning from patch 4) ?  
> > 
> > I was tried to not touch nla_nest_end() as it is used everywhere. But it makes
> > sense to me to add a new function to check this. I'm not very good at naming,
> > maybe `nla_nest_end_validate()` ? Or any other name if you have?
> 
> I was thinking nla_nest_end_check() to match the __must_check
> attribute. But I also like nla_nest_end_safe()
> _validate() is a little long but anything <=5 chars is fine.

OK, I will use nla_nest_end_safe() until we have a better name.