@@ -1198,6 +1198,46 @@ nl_dump_check_consistent(struct netlink_callback *cb,
cb->prev_seq = cb->seq;
}
+/**
+ * nl_dump_legacy_retval - get legacy return code for netlink dumps
+ * @err: error encountered during dump, negative errno or zero
+ * @skb: skb with the dump results
+ *
+ * Netlink dump callbacks get called multiple times per dump, because
+ * all the objects may not fit into a single skb. Whether another iteration
+ * is necessary gets decided based on the return value of the callback
+ * (with 0 meaning "end reached").
+ *
+ * The semantics used to be more complicated, with positive return values
+ * meaning "continue" and negative meaning "end with an error". A lot of
+ * handlers simplified this to return skb->len ? : -errno. Meaning that zero
+ * would only be returned when skb was empty, requiring another recvmsg()
+ * syscall just to get the NLM_DONE message.
+ *
+ * The current semantics allow handlers to also return -EMSGSIZE to continue.
+ *
+ * Unfortunately, some user space has started to depend on the NLM_DONE
+ * message being returned individually, in a separate recvmsg(). Select
+ * netlink dumps must preserve those semantics.
+ *
+ * This helper wraps the "legacy logic" and serves as an annotation for
+ * dumps which are known to require legacy handling.
+ *
+ * When used in combination with for_each_netdev_dump() - make sure to
+ * invalidate the ifindex when iteration is done. for_each_netdev_dump()
+ * does not move the iterator index "after" the last valid entry.
+ *
+ * NOTE: Do not use this helper in new code or dumps without legacy users!
+ *
+ * Return: return code to use for a dump handler
+ */
+static inline int nl_dump_legacy_retval(int err, const struct sk_buff *skb)
+{
+ if (err < 0 && err != -EMSGSIZE)
+ return err;
+ return skb->len;
+}
+
/**************************************************************************
* Netlink Attributes
**************************************************************************/
@@ -2284,8 +2284,11 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
ext_filter_mask, 0, NULL, 0,
netnsid, GFP_KERNEL);
if (err < 0)
- break;
+ goto done;
}
+ ctx->ifindex = INT_MAX;
+done:
+ err = nl_dump_legacy_retval(err, skb);
cb->seq = tgt_net->dev_base_seq;
nl_dump_check_consistent(cb, nlmsg_hdr(skb));
if (netnsid >= 0)
@@ -1911,7 +1911,10 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
if (err < 0)
goto done;
}
+ ctx->ifindex = INT_MAX;
done:
+ err = nl_dump_legacy_retval(err, skb);
+
if (fillargs.netnsid >= 0)
put_net(tgt_net);
rcu_read_unlock();
@@ -1051,10 +1051,7 @@ static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
}
}
- /* Don't let NLM_DONE coalesce into a message, even if it could.
- * Some user space expects NLM_DONE in a separate recv().
- */
- err = skb->len;
+ err = nl_dump_legacy_retval(err, skb);
out:
cb->args[1] = e;
Jaroslav reports Dell's OMSA Systems Management Data Engine expects NLM_DONE in a separate recvmsg(), so revert to the old behavior. This is the same kind of fix as we added in commit 460b0d33cf10 ("inet: bring NLM_DONE out to a separate recv() again") so wrap the logic into a helper, to make it easier to keep track of which dump handles we know to require legacy handling (and possibly one day let sockets opt into not doing this). Tested: ./cli.py --dbg-small-recv 4096 --spec netlink/specs/rt_addr.yaml \ --dump getaddr --json '{"ifa-family": 2}' ./cli.py --dbg-small-recv 4096 --spec netlink/specs/rt_route.yaml \ --dump getroute --json '{"rtm-family": 2}' ./cli.py --dbg-small-recv 4096 --spec netlink/specs/rt_link.yaml \ --dump getlink Fixes: 3e41af90767d ("rtnetlink: use xarray iterator to implement rtnl_dump_ifinfo()") Fixes: cdb2f80f1c10 ("inet: use xa_array iterator to implement inet_dump_ifaddr()") Reported-by: Jaroslav Pulchart <jaroslav.pulchart@gooddata.com> Link: https://lore.kernel.org/all/CAK8fFZ7MKoFSEzMBDAOjoUt+vTZRRQgLDNXEOfdCCXSoXXKE0g@mail.gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- v2: - adjust the comment in the kdoc - add getlink CC: dsahern@kernel.org --- include/net/netlink.h | 40 ++++++++++++++++++++++++++++++++++++++++ net/core/rtnetlink.c | 5 ++++- net/ipv4/devinet.c | 3 +++ net/ipv4/fib_frontend.c | 5 +---- 4 files changed, 48 insertions(+), 5 deletions(-)