* [PATCH net] rtnetlink: truncate IFLA_VFINFO_LIST instead of overflowing its length
@ 2026-07-25 13:22 Artem Lytkin
2026-07-30 1:55 ` Jakub Kicinski
0 siblings, 1 reply; 7+ messages in thread
From: Artem Lytkin @ 2026-07-25 13:22 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, jacob.e.keller, chrisw,
linux-kernel
rtnl_fill_vf() closes the IFLA_VFINFO_LIST nest with nla_nest_end(),
which stores the accumulated length into nla_len. That field is a u16,
so a nest larger than 65535 bytes is written truncated modulo 65536.
The skb is sized by if_nlmsg_size(), which adds rtnl_vfinfo_size() for
every VF, so the buffer really is large enough for the whole list and
none of the nla_put() calls in rtnl_fill_vfinfo() fails. The overflow is
therefore silent. Userspace then walks the message with RTA_NEXT(),
which advances by the stored length, so parsing resumes inside VF
payload and the top-level attributes that follow the nest are read out
of VF data. Those are IFLA_VF_PORTS, IFLA_XDP, IFLA_LINKINFO,
IFLA_PERM_ADDRESS and IFLA_PROP_LIST. iproute2 prints "!!!Deficit", and
strictly validating parsers reject the message outright.
rtnl_fill_vfinfo() emits 296 bytes per VF on a 64-bit kernel with
efficient unaligned access, so the nest wraps at 222 VFs; without
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS it is 328 bytes and 200 VFs, and
with ndo_get_vf_guid 336 bytes and 196 VFs. ice supports up to 256 VFs
per PF (ICE_MAX_SRIOV_VFS), so this is reachable on shipping hardware.
Requests that set RTEXT_FILTER_SKIP_STATS need 335 VFs, which the 256-VF
limit puts out of reach, so plain "ip link show" is fine today while
"ip -s link show" is not.
On CONFIG_DEBUG_NET kernels nla_nest_end() now also splats via
DEBUG_NET_WARN_ON_ONCE(), added along with nla_nest_end_safe() in commit
1346586a9ac9 ("netlink: add a nla_nest_end_safe() helper").
Using nla_nest_end_safe() here would not help: a nest that does not fit
in a u16 will not fit in a retried skb either, so returning -EMSGSIZE
would turn "ip link show" on such a device into a hard failure. Bound
the nest in the writer instead, as suggested when this was last
discussed: drop the VF that would push it past U16_MAX and end the list
there. Userspace sees IFLA_NUM_VF unchanged and a shorter
IFLA_VFINFO_LIST, and everything after the nest stays parsable. An empty
nest is already emitted for a PF with no VFs, so a list shorter than
IFLA_NUM_VF is not a new encoding.
The other large nests in rtnl_fill_ifinfo() were audited and cannot
overflow: IFLA_AF_SPEC is bounded by a handful of address families at
about a kilobyte each, and IFLA_VF_PORTS would need more than 560 VFs.
Fixes: c02db8c6290b ("rtnetlink: make SR-IOV VF interface symmetric")
Reported-by: Jacob Keller <jacob.e.keller@intel.com>
Closes: https://lore.kernel.org/netdev/16b289f6-b025-5dd3-443d-92d4c167e79c@intel.com/
Assisted-by: Claude:claude-fable-5
Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
net/core/rtnetlink.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 12aa3aa1688b1..bd1d65dcf42fd 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1687,10 +1687,22 @@ static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
return -EMSGSIZE;
for (i = 0; i < num_vfs; i++) {
+ unsigned char *mark = skb_tail_pointer(skb);
+
if (rtnl_fill_vfinfo(skb, dev, i, ext_filter_mask)) {
nla_nest_cancel(skb, vfinfo);
return -EMSGSIZE;
}
+
+ /* An attribute length is a u16, so the nest cannot describe
+ * more than U16_MAX bytes. Drop the VF that would overflow it
+ * and stop: a truncated list keeps the rest of the message
+ * parsable, whereas a wrapped nest length does not.
+ */
+ if (skb_tail_pointer(skb) - (unsigned char *)vfinfo > U16_MAX) {
+ nlmsg_trim(skb, mark);
+ break;
+ }
}
nla_nest_end(skb, vfinfo);
base-commit: 248951ddc14de84de3910f9b13f51491a8cd91df
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net] rtnetlink: truncate IFLA_VFINFO_LIST instead of overflowing its length 2026-07-25 13:22 [PATCH net] rtnetlink: truncate IFLA_VFINFO_LIST instead of overflowing its length Artem Lytkin @ 2026-07-30 1:55 ` Jakub Kicinski 2026-07-30 9:07 ` [PATCH net-next v2] rtnetlink: cap IFLA_VFINFO_LIST at a documented number of VFs Artem Lytkin 2026-07-30 9:17 ` [PATCH net] rtnetlink: truncate IFLA_VFINFO_LIST instead of overflowing its length Jacob Keller 0 siblings, 2 replies; 7+ messages in thread From: Jakub Kicinski @ 2026-07-30 1:55 UTC (permalink / raw) To: Artem Lytkin Cc: netdev, davem, edumazet, pabeni, horms, jacob.e.keller, chrisw, linux-kernel On Sat, 25 Jul 2026 16:22:36 +0300 Artem Lytkin wrote: > rtnl_fill_vf() closes the IFLA_VFINFO_LIST nest with nla_nest_end(), > which stores the accumulated length into nla_len. That field is a u16, > so a nest larger than 65535 bytes is written truncated modulo 65536. > > The skb is sized by if_nlmsg_size(), which adds rtnl_vfinfo_size() for > every VF, so the buffer really is large enough for the whole list and > none of the nla_put() calls in rtnl_fill_vfinfo() fails. The overflow is > therefore silent. Userspace then walks the message with RTA_NEXT(), > which advances by the stored length, so parsing resumes inside VF > payload and the top-level attributes that follow the nest are read out > of VF data. Those are IFLA_VF_PORTS, IFLA_XDP, IFLA_LINKINFO, > IFLA_PERM_ADDRESS and IFLA_PROP_LIST. iproute2 prints "!!!Deficit", and > strictly validating parsers reject the message outright. > > rtnl_fill_vfinfo() emits 296 bytes per VF on a 64-bit kernel with > efficient unaligned access, so the nest wraps at 222 VFs; without > CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS it is 328 bytes and 200 VFs, and > with ndo_get_vf_guid 336 bytes and 196 VFs. ice supports up to 256 VFs > per PF (ICE_MAX_SRIOV_VFS), so this is reachable on shipping hardware. > Requests that set RTEXT_FILTER_SKIP_STATS need 335 VFs, which the 256-VF > limit puts out of reach, so plain "ip link show" is fine today while > "ip -s link show" is not. We should cap the output at known values and document them as the max the interface supports. "The number depends on attr set and random things like CPU alignment requirements" sounds like a terrible uAPI contract. Perhaps 128 with stats and 256 without? > On CONFIG_DEBUG_NET kernels nla_nest_end() now also splats via > DEBUG_NET_WARN_ON_ONCE(), added along with nla_nest_end_safe() in commit > 1346586a9ac9 ("netlink: add a nla_nest_end_safe() helper"). > > Using nla_nest_end_safe() here would not help: a nest that does not fit > in a u16 will not fit in a retried skb either, so returning -EMSGSIZE > would turn "ip link show" on such a device into a hard failure. Bound > the nest in the writer instead, as suggested when this was last > discussed: drop the VF that would push it past U16_MAX and end the list > there. Userspace sees IFLA_NUM_VF unchanged and a shorter > IFLA_VFINFO_LIST, and everything after the nest stays parsable. An empty > nest is already emitted for a PF with no VFs, so a list shorter than > IFLA_NUM_VF is not a new encoding. > > The other large nests in rtnl_fill_ifinfo() were audited and cannot > overflow: IFLA_AF_SPEC is bounded by a handful of address families at > about a kilobyte each, and IFLA_VF_PORTS would need more than 560 VFs. > > Fixes: c02db8c6290b ("rtnetlink: make SR-IOV VF interface symmetric") Not a fix - this is along standing limitation of the interface. -- pw-bot: cr ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next v2] rtnetlink: cap IFLA_VFINFO_LIST at a documented number of VFs 2026-07-30 1:55 ` Jakub Kicinski @ 2026-07-30 9:07 ` Artem Lytkin 2026-07-30 21:09 ` Jakub Kicinski 2026-07-30 9:17 ` [PATCH net] rtnetlink: truncate IFLA_VFINFO_LIST instead of overflowing its length Jacob Keller 1 sibling, 1 reply; 7+ messages in thread From: Artem Lytkin @ 2026-07-30 9:07 UTC (permalink / raw) To: netdev Cc: kuba, davem, edumazet, pabeni, horms, donald.hunter, jacob.e.keller, linux-kernel rtnl_fill_vf() emits one IFLA_VF_INFO per VF into the IFLA_VFINFO_LIST nest and closes it with nla_nest_end(), which stores the accumulated length into nla_len. That field is a u16, so a nest larger than 65535 bytes is written truncated modulo 65536. Nothing catches it on the way. if_nlmsg_size() adds rtnl_vfinfo_size() for every VF, so the skb really is large enough and none of the nla_put() calls fails. Userspace then walks the message with RTA_NEXT(), which advances by the stored length, so parsing resumes inside VF payload and the attributes after the nest are read out of VF data: IFLA_VF_PORTS, IFLA_XDP, IFLA_LINKINFO, IFLA_PERM_ADDRESS, IFLA_AF_SPEC. iproute2 prints "!!!Deficit" and strictly validating parsers reject the message. On CONFIG_DEBUG_NET kernels nla_nest_end() also splats, via the DEBUG_NET_WARN_ON_ONCE() added in commit 1346586a9ac9 ("netlink: add a nla_nest_end_safe() helper"). Where the wrap falls depends on what was asked for and on the host. A VF costs 196 bytes, 296 with statistics, 236 with GUIDs and 336 with both. On a kernel without CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS the statistics carry a padding attribute each and cost 32 bytes more, which makes those two 328 and 368. So the nest overflows somewhere between 179 and 335 VFs. ice allows 256 VFs per PF (ICE_MAX_SRIOV_VFS), which reaches it. Statistics are included unless the request sets RTEXT_FILTER_SKIP_STATS, so the common case is the one that wraps first. A limit that moves with the requested attribute set and with the host's alignment requirements is not something userspace can be told, so use fixed numbers instead and document them as what the interface supports: 256 VFs, or 128 when statistics are included. Both stay well inside U16_MAX even in the largest per-VF encoding, at 60416 and 47104 bytes respectively. rtnl_vfinfo_size() applies the same limit so that the skb is not sized for VFs that will not be emitted. A device with more VFs than the limit reports a shorter IFLA_VFINFO_LIST. IFLA_NUM_VF keeps carrying the real count, and everything after the nest stays parsable, which is the part that is broken today. An empty nest is already emitted for a PF with no VFs, so a list shorter than IFLA_NUM_VF is not a new encoding. Returning -EMSGSIZE instead, which is what nla_nest_end_safe() would give, is not an option here: a nest that does not fit in a u16 will not fit in a retried skb either, so it would turn a link dump on such a device into a hard failure. The other large nests in rtnl_fill_ifinfo() were audited and cannot overflow. IFLA_AF_SPEC is bounded by a handful of address families at about a kilobyte each, and IFLA_VF_PORTS would need more than 560 VFs, which no in-tree driver allows. Reported-by: Jacob Keller <jacob.e.keller@intel.com> Link: https://lore.kernel.org/netdev/16b289f6-b025-5dd3-443d-92d4c167e79c@intel.com/ Assisted-by: Claude:claude-fable-5 Signed-off-by: Artem Lytkin <iprintercanon@gmail.com> --- v2: - cap at fixed limits, 256 VFs and 128 when statistics are included, and document them in the netlink spec, rather than dropping whichever VF happens not to fit (Jakub) - drop the Fixes: tag, this is a long standing limitation of the interface and not a regression (Jakub) - retarget net-next for the same reason - changelog: corrected the per-VF sizes, which mixed two configurations, and dropped IFLA_PROP_LIST from the list of attributes that follow the nest, since it is emitted before it v1: https://lore.kernel.org/netdev/20260725132236.88318-1-iprintercanon@gmail.com/ Documentation/netlink/specs/rt-link.yaml | 5 +++++ net/core/rtnetlink.c | 25 +++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/Documentation/netlink/specs/rt-link.yaml b/Documentation/netlink/specs/rt-link.yaml index 68c26a70bb649..b80c2ac3ac311 100644 --- a/Documentation/netlink/specs/rt-link.yaml +++ b/Documentation/netlink/specs/rt-link.yaml @@ -928,6 +928,11 @@ attribute-sets: name: vfinfo-list type: nest nested-attributes: vfinfo-list-attrs + doc: | + Per-VF details. The list holds at most 256 VFs, or 128 when + statistics are included, because it is one attribute and has to fit + in a u16 length. A device with more VFs than that reports a + truncated list; num-vf still carries the real count. - name: stats64 type: binary diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 31c65a545a107..6395ea6701210 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -1174,12 +1174,30 @@ static void copy_rtnl_link_stats(struct rtnl_link_stats *a, a->rx_nohandler = b->rx_nohandler; } +/* IFLA_VFINFO_LIST is one netlink attribute, so everything nested inside it + * has to fit in the u16 nla_len. Bound the number of VFs described there at a + * fixed value rather than at whatever happens to fit, so that the limit is a + * property of the interface instead of one of the requested attribute set and + * the host's alignment requirements. The largest per-VF encoding is 368 bytes + * with statistics and GUIDs and 236 bytes without statistics, so both limits + * keep the nest well inside U16_MAX. + */ +#define RTNL_VFINFO_MAX_VFS 256 +#define RTNL_VFINFO_MAX_VFS_STATS 128 + +static int rtnl_vfinfo_max_vfs(u32 ext_filter_mask) +{ + return ext_filter_mask & RTEXT_FILTER_SKIP_STATS ? + RTNL_VFINFO_MAX_VFS : RTNL_VFINFO_MAX_VFS_STATS; +} + /* All VF info */ static inline int rtnl_vfinfo_size(const struct net_device *dev, u32 ext_filter_mask) { if (dev->dev.parent && (ext_filter_mask & RTEXT_FILTER_VF)) { - int num_vfs = dev_num_vf(dev->dev.parent); + int num_vfs = min(dev_num_vf(dev->dev.parent), + rtnl_vfinfo_max_vfs(ext_filter_mask)); size_t size = nla_total_size(0); size += num_vfs * (nla_total_size(0) + @@ -1717,6 +1735,11 @@ static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb, if (!vfinfo) return -EMSGSIZE; + /* IFLA_NUM_VF above stays the device's VF count; the list itself is + * capped so that its length cannot overflow nla_len. + */ + num_vfs = min(num_vfs, rtnl_vfinfo_max_vfs(ext_filter_mask)); + for (i = 0; i < num_vfs; i++) { if (rtnl_fill_vfinfo(skb, dev, i, ext_filter_mask)) { nla_nest_cancel(skb, vfinfo); base-commit: bc03e080390a1b8dd6252b598aa5ec801fc5e13d -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] rtnetlink: cap IFLA_VFINFO_LIST at a documented number of VFs 2026-07-30 9:07 ` [PATCH net-next v2] rtnetlink: cap IFLA_VFINFO_LIST at a documented number of VFs Artem Lytkin @ 2026-07-30 21:09 ` Jakub Kicinski 2026-07-30 23:14 ` Jacob Keller 0 siblings, 1 reply; 7+ messages in thread From: Jakub Kicinski @ 2026-07-30 21:09 UTC (permalink / raw) To: Artem Lytkin Cc: netdev, davem, edumazet, pabeni, horms, donald.hunter, jacob.e.keller, linux-kernel On Thu, 30 Jul 2026 12:07:00 +0300 Artem Lytkin wrote: > rtnl_fill_vf() emits one IFLA_VF_INFO per VF into the IFLA_VFINFO_LIST > nest and closes it with nla_nest_end(), which stores the accumulated > length into nla_len. That field is a u16, so a nest larger than 65535 > bytes is written truncated modulo 65536. > > Nothing catches it on the way. if_nlmsg_size() adds rtnl_vfinfo_size() > for every VF, so the skb really is large enough and none of the nla_put() > calls fails. Userspace then walks the message with RTA_NEXT(), which > advances by the stored length, so parsing resumes inside VF payload and > the attributes after the nest are read out of VF data: IFLA_VF_PORTS, > IFLA_XDP, IFLA_LINKINFO, IFLA_PERM_ADDRESS, IFLA_AF_SPEC. iproute2 > prints "!!!Deficit" and strictly validating parsers reject the message. > On CONFIG_DEBUG_NET kernels nla_nest_end() also splats, via the > DEBUG_NET_WARN_ON_ONCE() added in commit 1346586a9ac9 ("netlink: add a > nla_nest_end_safe() helper"). > > Where the wrap falls depends on what was asked for and on the host. A VF > costs 196 bytes, 296 with statistics, 236 with GUIDs and 336 with both. > On a kernel without CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS the statistics > carry a padding attribute each and cost 32 bytes more, which makes those > two 328 and 368. So the nest overflows somewhere between 179 and 335 > VFs. ice allows 256 VFs per PF > (ICE_MAX_SRIOV_VFS), which reaches it. Statistics are included unless the > request sets RTEXT_FILTER_SKIP_STATS, so the common case is the one that > wraps first. > > A limit that moves with the requested attribute set and with the host's > alignment requirements is not something userspace can be told, so use > fixed numbers instead and document them as what the interface supports: > 256 VFs, or 128 when statistics are included. Both stay well inside > U16_MAX even in the largest per-VF encoding, at 60416 and 47104 bytes > respectively. rtnl_vfinfo_size() applies the same limit so that the skb > is not sized for VFs that will not be emitted. > > A device with more VFs than the limit reports a shorter > IFLA_VFINFO_LIST. IFLA_NUM_VF keeps carrying the real count, and > everything after the nest stays parsable, which is the part that is > broken today. An empty nest is already emitted for a PF with no VFs, so a > list shorter than IFLA_NUM_VF is not a new encoding. > > Returning -EMSGSIZE instead, which is what nla_nest_end_safe() would > give, is not an option here: a nest that does not fit in a u16 will not > fit in a retried skb either, so it would turn a link dump on such a > device into a hard failure. > > The other large nests in rtnl_fill_ifinfo() were audited and cannot > overflow. IFLA_AF_SPEC is bounded by a handful of address families at > about a kilobyte each, and IFLA_VF_PORTS would need more than 560 VFs, > which no in-tree driver allows. > > Reported-by: Jacob Keller <jacob.e.keller@intel.com> > Link: https://lore.kernel.org/netdev/16b289f6-b025-5dd3-443d-92d4c167e79c@intel.com/ > Assisted-by: Claude:claude-fable-5 > Signed-off-by: Artem Lytkin <iprintercanon@gmail.com> > --- > v2: > - cap at fixed limits, 256 VFs and 128 when statistics are included, and > document them in the netlink spec, rather than dropping whichever VF > happens not to fit (Jakub) > - drop the Fixes: tag, this is a long standing limitation of the > interface and not a regression (Jakub) > - retarget net-next for the same reason > - changelog: corrected the per-VF sizes, which mixed two configurations, > and dropped IFLA_PROP_LIST from the list of attributes that follow the > nest, since it is emitted before it > v1: https://lore.kernel.org/netdev/20260725132236.88318-1-iprintercanon@gmail.com/ > > Documentation/netlink/specs/rt-link.yaml | 5 +++++ > net/core/rtnetlink.c | 25 +++++++++++++++++++++++- > 2 files changed, 29 insertions(+), 1 deletion(-) > > diff --git a/Documentation/netlink/specs/rt-link.yaml b/Documentation/netlink/specs/rt-link.yaml > index 68c26a70bb649..b80c2ac3ac311 100644 > --- a/Documentation/netlink/specs/rt-link.yaml > +++ b/Documentation/netlink/specs/rt-link.yaml > @@ -928,6 +928,11 @@ attribute-sets: > name: vfinfo-list > type: nest > nested-attributes: vfinfo-list-attrs > + doc: | > + Per-VF details. The list holds at most 256 VFs, or 128 when > + statistics are included, because it is one attribute and has to fit > + in a u16 length. A device with more VFs than that reports a > + truncated list; num-vf still carries the real count. > - > name: stats64 > type: binary > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index 31c65a545a107..6395ea6701210 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -1174,12 +1174,30 @@ static void copy_rtnl_link_stats(struct rtnl_link_stats *a, > a->rx_nohandler = b->rx_nohandler; > } > > +/* IFLA_VFINFO_LIST is one netlink attribute, so everything nested inside it > + * has to fit in the u16 nla_len. Bound the number of VFs described there at a > + * fixed value rather than at whatever happens to fit, so that the limit is a > + * property of the interface instead of one of the requested attribute set and > + * the host's alignment requirements. The largest per-VF encoding is 368 bytes > + * with statistics and GUIDs and 236 bytes without statistics, so both limits > + * keep the nest well inside U16_MAX. > + */ > +#define RTNL_VFINFO_MAX_VFS 256 > +#define RTNL_VFINFO_MAX_VFS_STATS 128 No strong opinion which to choose but I'd either move these defines somewhere visible or just use the literal values. There's a common fallacy that all literals must have defines, IMO if the literal is only used once, by a small well scoped helper (here rtnl_vfinfo_max_vfs()) there's no ambiguity. The define is an extra indirection obfuscating the code. If you don't care either way either - I'd do the literal thing, and let the comment describe the helper > +static int rtnl_vfinfo_max_vfs(u32 ext_filter_mask) if you pass the "real" num_vfs in here you can also subsume the min() into the helper. Perhaps the helper should then be renamed s/_max/_cap/ > +{ > + return ext_filter_mask & RTEXT_FILTER_SKIP_STATS ? > + RTNL_VFINFO_MAX_VFS : RTNL_VFINFO_MAX_VFS_STATS; > +} Other than that LGTM. Please send v3 as a new thread, not in reply to. And FWIW (sorry, Im going to reply here to avoid splintering) I maintain that net-next is better :) We are capping, who knows is someone complains. So I _don't_ want this in LTS. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] rtnetlink: cap IFLA_VFINFO_LIST at a documented number of VFs 2026-07-30 21:09 ` Jakub Kicinski @ 2026-07-30 23:14 ` Jacob Keller 0 siblings, 0 replies; 7+ messages in thread From: Jacob Keller @ 2026-07-30 23:14 UTC (permalink / raw) To: Jakub Kicinski, Artem Lytkin Cc: netdev, davem, edumazet, pabeni, horms, donald.hunter, linux-kernel On 7/30/2026 2:09 PM, Jakub Kicinski wrote: > And FWIW (sorry, Im going to reply here to avoid splintering) I maintain > that net-next is better :) We are capping, who knows is someone > complains. So I _don't_ want this in LTS. > Fair enough, no objections here. I'd appreciate something in the commit message mentioning the commit this originated in, since that may still be helpful to future readers for context, but I won't hold an objection regardless of what ends up in v3. Thanks, Jake ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] rtnetlink: truncate IFLA_VFINFO_LIST instead of overflowing its length 2026-07-30 1:55 ` Jakub Kicinski 2026-07-30 9:07 ` [PATCH net-next v2] rtnetlink: cap IFLA_VFINFO_LIST at a documented number of VFs Artem Lytkin @ 2026-07-30 9:17 ` Jacob Keller 2026-07-30 17:12 ` Artem Lytkin 1 sibling, 1 reply; 7+ messages in thread From: Jacob Keller @ 2026-07-30 9:17 UTC (permalink / raw) To: Jakub Kicinski, Artem Lytkin Cc: netdev, davem, edumazet, pabeni, horms, chrisw, linux-kernel On 7/29/2026 6:55 PM, Jakub Kicinski wrote: > On Sat, 25 Jul 2026 16:22:36 +0300 Artem Lytkin wrote: >> rtnl_fill_vf() closes the IFLA_VFINFO_LIST nest with nla_nest_end(), >> which stores the accumulated length into nla_len. That field is a u16, >> so a nest larger than 65535 bytes is written truncated modulo 65536. >> >> The skb is sized by if_nlmsg_size(), which adds rtnl_vfinfo_size() for >> every VF, so the buffer really is large enough for the whole list and >> none of the nla_put() calls in rtnl_fill_vfinfo() fails. The overflow is >> therefore silent. Userspace then walks the message with RTA_NEXT(), >> which advances by the stored length, so parsing resumes inside VF >> payload and the top-level attributes that follow the nest are read out >> of VF data. Those are IFLA_VF_PORTS, IFLA_XDP, IFLA_LINKINFO, >> IFLA_PERM_ADDRESS and IFLA_PROP_LIST. iproute2 prints "!!!Deficit", and >> strictly validating parsers reject the message outright. >> >> rtnl_fill_vfinfo() emits 296 bytes per VF on a 64-bit kernel with >> efficient unaligned access, so the nest wraps at 222 VFs; without >> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS it is 328 bytes and 200 VFs, and >> with ndo_get_vf_guid 336 bytes and 196 VFs. ice supports up to 256 VFs >> per PF (ICE_MAX_SRIOV_VFS), so this is reachable on shipping hardware. >> Requests that set RTEXT_FILTER_SKIP_STATS need 335 VFs, which the 256-VF >> limit puts out of reach, so plain "ip link show" is fine today while >> "ip -s link show" is not. > > We should cap the output at known values and document them as the max > the interface supports. "The number depends on attr set and random > things like CPU alignment requirements" sounds like a terrible uAPI > contract. > > Perhaps 128 with stats and 256 without? > This seems reasonable to me. >> On CONFIG_DEBUG_NET kernels nla_nest_end() now also splats via >> DEBUG_NET_WARN_ON_ONCE(), added along with nla_nest_end_safe() in commit >> 1346586a9ac9 ("netlink: add a nla_nest_end_safe() helper"). >> >> Using nla_nest_end_safe() here would not help: a nest that does not fit >> in a u16 will not fit in a retried skb either, so returning -EMSGSIZE >> would turn "ip link show" on such a device into a hard failure. Bound >> the nest in the writer instead, as suggested when this was last >> discussed: drop the VF that would push it past U16_MAX and end the list >> there. Userspace sees IFLA_NUM_VF unchanged and a shorter >> IFLA_VFINFO_LIST, and everything after the nest stays parsable. An empty >> nest is already emitted for a PF with no VFs, so a list shorter than >> IFLA_NUM_VF is not a new encoding. >> >> The other large nests in rtnl_fill_ifinfo() were audited and cannot >> overflow: IFLA_AF_SPEC is bounded by a handful of address families at >> about a kilobyte each, and IFLA_VF_PORTS would need more than 560 VFs. >> >> Fixes: c02db8c6290b ("rtnetlink: make SR-IOV VF interface symmetric") > > Not a fix - this is along standing limitation of the interface. I'd argue this is a "fix", as we were previously doing objectively the wrong thing by randomly truncating the message at arbitrary points. That being said, this *is* a well known limitation of the interface that has been around for a while, so I can accept the argument not to tag it as a fix for net. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] rtnetlink: truncate IFLA_VFINFO_LIST instead of overflowing its length 2026-07-30 9:17 ` [PATCH net] rtnetlink: truncate IFLA_VFINFO_LIST instead of overflowing its length Jacob Keller @ 2026-07-30 17:12 ` Artem Lytkin 0 siblings, 0 replies; 7+ messages in thread From: Artem Lytkin @ 2026-07-30 17:12 UTC (permalink / raw) To: Jacob Keller, Jakub Kicinski Cc: netdev, davem, edumazet, pabeni, horms, chrisw, linux-kernel On Thu, Jul 30, 2026 at 02:17:35AM -0700, Jacob Keller wrote: > I'd argue this is a "fix", as we were previously doing objectively the > wrong thing by randomly truncating the message at arbitrary points. Our mails crossed. v2 went out ten minutes before yours, taking Jakub's 128 and 256, documenting them in the netlink spec and dropping the tag: https://lore.kernel.org/netdev/20260730090700.65867-1-iprintercanon@gmail.com/ I have no strong view on who is right, but the tag decides more than bookkeeping here. With it this goes to net and gets backported, so an ice PF with 256 VFs stops producing a corrupt link dump on a stable kernel; without it that waits for the merge window. Against that, nothing regressed: the interface has been able to write a length it cannot represent since 2010, which I take to be Jakub's point. Happy to restore the tag and resend against net if Jakub is persuaded. Artem ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-07-30 23:15 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-25 13:22 [PATCH net] rtnetlink: truncate IFLA_VFINFO_LIST instead of overflowing its length Artem Lytkin 2026-07-30 1:55 ` Jakub Kicinski 2026-07-30 9:07 ` [PATCH net-next v2] rtnetlink: cap IFLA_VFINFO_LIST at a documented number of VFs Artem Lytkin 2026-07-30 21:09 ` Jakub Kicinski 2026-07-30 23:14 ` Jacob Keller 2026-07-30 9:17 ` [PATCH net] rtnetlink: truncate IFLA_VFINFO_LIST instead of overflowing its length Jacob Keller 2026-07-30 17:12 ` Artem Lytkin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.