git: b977dd1ea5fb - main - linux: make linux_netlink_p->msg_from_linux be able to fail

From: Gleb Smirnoff <glebius_at_FreeBSD.org>
Date: Fri, 29 Mar 2024 20:36:13 UTC
The branch main has been updated by glebius:

URL: https://cgit.FreeBSD.org/src/commit/?id=b977dd1ea5fbc2df3f1279330be4d089322eb2cf

commit b977dd1ea5fbc2df3f1279330be4d089322eb2cf
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2024-03-29 20:35:51 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2024-03-29 20:35:51 +0000

    linux: make linux_netlink_p->msg_from_linux be able to fail
    
    The KPI for this function was misleading.  From the NetLink perspective it
    looked like a function that: a) allocates new hdr, b) can fail.  Neither
    was true.  Let the function return a error code instead of returning the
    same hdr it was passed to.  In case if future Linux NetLink compatibility
    support calls for reallocating header, pass hdr as pointer to pointer.
    
    With KPI that returns a error, propagate domain conversion errors all the
    way up to NetLink module.  This fixes panic when unknown domain is
    converted to 0xff and this invalid value is passed into NetLink
    processing.
    
    PR:                     274536
    Reviewed by:            melifaro
    Differential Revision:  https://reviews.freebsd.org/D44392
---
 sys/compat/linux/linux_netlink.c | 58 ++++++++++++++++++++++++++--------------
 sys/netlink/netlink_io.c         | 22 +++++++--------
 sys/netlink/netlink_linux.h      |  2 +-
 3 files changed, 48 insertions(+), 34 deletions(-)

diff --git a/sys/compat/linux/linux_netlink.c b/sys/compat/linux/linux_netlink.c
index d2afec24fe71..8675f830b4ef 100644
--- a/sys/compat/linux/linux_netlink.c
+++ b/sys/compat/linux/linux_netlink.c
@@ -31,7 +31,6 @@
 #include <sys/types.h>
 #include <sys/ck.h>
 #include <sys/lock.h>
-#include <sys/malloc.h>
 #include <sys/socket.h>
 #include <sys/vnode.h>
 
@@ -73,37 +72,55 @@ _rta_get_uint32(const struct rtattr *rta)
 	return (*((const uint32_t *)NL_RTA_DATA_CONST(rta)));
 }
 
-static struct nlmsghdr *
+static int
 rtnl_neigh_from_linux(struct nlmsghdr *hdr, struct nl_pstate *npt)
 {
 	struct ndmsg *ndm = (struct ndmsg *)(hdr + 1);
+	sa_family_t f;
+
+	if (hdr->nlmsg_len < sizeof(struct nlmsghdr) + sizeof(struct ndmsg))
+		return (EBADMSG);
+	if ((f = linux_to_bsd_domain(ndm->ndm_family)) == AF_UNKNOWN)
+		return (EPFNOSUPPORT);
 
-	if (hdr->nlmsg_len >= sizeof(struct nlmsghdr) + sizeof(struct ndmsg))
-		ndm->ndm_family = linux_to_bsd_domain(ndm->ndm_family);
+	ndm->ndm_family = f;
 
-	return (hdr);
+	return (0);
 }
 
-static struct nlmsghdr *
+static int
 rtnl_ifaddr_from_linux(struct nlmsghdr *hdr, struct nl_pstate *npt)
 {
 	struct ifaddrmsg *ifam = (struct ifaddrmsg *)(hdr + 1);
+	sa_family_t f;
 
-	if (hdr->nlmsg_len >= sizeof(struct nlmsghdr) + sizeof(struct ifaddrmsg))
-		ifam->ifa_family = linux_to_bsd_domain(ifam->ifa_family);
+	if (hdr->nlmsg_len < sizeof(struct nlmsghdr) + sizeof(struct ifaddrmsg))
+		return (EBADMSG);
+	if ((f = linux_to_bsd_domain(ifam->ifa_family)) == AF_UNKNOWN)
+		return (EPFNOSUPPORT);
 
-	return (hdr);
+	ifam->ifa_family = f;
+
+	return (0);
 }
 
-static struct nlmsghdr *
+/*
+ * XXX: in case of error state of hdr is inconsistent.
+ */
+static int
 rtnl_route_from_linux(struct nlmsghdr *hdr, struct nl_pstate *npt)
 {
 	/* Tweak address families and default fib only */
 	struct rtmsg *rtm = (struct rtmsg *)(hdr + 1);
 	struct nlattr *nla, *nla_head;
 	int attrs_len;
+	sa_family_t f;
 
-	rtm->rtm_family = linux_to_bsd_domain(rtm->rtm_family);
+	if (hdr->nlmsg_len < sizeof(struct nlmsghdr) + sizeof(struct rtmsg))
+		return (EBADMSG);
+	if ((f = linux_to_bsd_domain(rtm->rtm_family)) == AF_UNKNOWN)
+		return (EPFNOSUPPORT);
+	rtm->rtm_family = f;
 
 	if (rtm->rtm_table == 254)
 		rtm->rtm_table = 0;
@@ -122,7 +139,7 @@ rtnl_route_from_linux(struct nlmsghdr *hdr, struct nl_pstate *npt)
 		switch (rta->rta_type) {
 		case NL_RTA_TABLE:
 			if (!valid_rta_u32(rta))
-				goto done;
+				return (EBADMSG);
 			rtm->rtm_table = 0;
 			uint32_t fibnum = _rta_get_uint32(rta);
 			RT_LOG(LOG_DEBUG3, "GET RTABLE: %u", fibnum);
@@ -133,13 +150,13 @@ rtnl_route_from_linux(struct nlmsghdr *hdr, struct nl_pstate *npt)
 		}
 	}
 
-done:
-	return (hdr);
+	return (0);
 }
 
-static struct nlmsghdr *
+static int
 rtnl_from_linux(struct nlmsghdr *hdr, struct nl_pstate *npt)
 {
+
 	switch (hdr->nlmsg_type) {
 	case NL_RTM_GETROUTE:
 	case NL_RTM_NEWROUTE:
@@ -157,21 +174,22 @@ rtnl_from_linux(struct nlmsghdr *hdr, struct nl_pstate *npt)
 	default:
 		RT_LOG(LOG_DEBUG, "Passing message type %d untranslated",
 		    hdr->nlmsg_type);
+		/* XXXGL: maybe return error? */
 	}
 
-	return (hdr);
+	return (0);
 }
 
-static struct nlmsghdr *
-nlmsg_from_linux(int netlink_family, struct nlmsghdr *hdr,
+static int
+nlmsg_from_linux(int netlink_family, struct nlmsghdr **hdr,
     struct nl_pstate *npt)
 {
 	switch (netlink_family) {
 	case NETLINK_ROUTE:
-		return (rtnl_from_linux(hdr, npt));
+		return (rtnl_from_linux(*hdr, npt));
 	}
 
-	return (hdr);
+	return (0);
 }
 
 
diff --git a/sys/netlink/netlink_io.c b/sys/netlink/netlink_io.c
index 61d9d657556a..2dd49d5e8eb5 100644
--- a/sys/netlink/netlink_io.c
+++ b/sys/netlink/netlink_io.c
@@ -275,25 +275,21 @@ nl_receive_message(struct nlmsghdr *hdr, int remaining_length,
 
 	npt->hdr = hdr;
 
-	if (hdr->nlmsg_flags & NLM_F_REQUEST && hdr->nlmsg_type >= NLMSG_MIN_TYPE) {
+	if (hdr->nlmsg_flags & NLM_F_REQUEST &&
+	    hdr->nlmsg_type >= NLMSG_MIN_TYPE) {
 		NL_LOG(LOG_DEBUG2, "handling message with msg type: %d",
 		   hdr->nlmsg_type);
-
-		if (nlp->nl_linux && linux_netlink_p != NULL) {
-			struct nlmsghdr *hdr_orig = hdr;
-			hdr = linux_netlink_p->msg_from_linux(nlp->nl_proto, hdr, npt);
-			if (hdr == NULL) {
-				 /* Failed to translate to kernel format. Report an error back */
-				hdr = hdr_orig;
-				npt->hdr = hdr;
-				if (hdr->nlmsg_flags & NLM_F_ACK)
-					nlmsg_ack(nlp, EOPNOTSUPP, hdr, npt);
-				return (0);
-			}
+		if (nlp->nl_linux) {
+			MPASS(linux_netlink_p != NULL);
+			error = linux_netlink_p->msg_from_linux(nlp->nl_proto,
+			    &hdr, npt);
+			if (error)
+				goto ack;
 		}
 		error = handler(hdr, npt);
 		NL_LOG(LOG_DEBUG2, "retcode: %d", error);
 	}
+ack:
 	if ((hdr->nlmsg_flags & NLM_F_ACK) || (error != 0 && error != EINTR)) {
 		if (!npt->nw->suppress_ack) {
 			NL_LOG(LOG_DEBUG3, "ack");
diff --git a/sys/netlink/netlink_linux.h b/sys/netlink/netlink_linux.h
index 2d9f8d1b7bd6..d4c451d470b2 100644
--- a/sys/netlink/netlink_linux.h
+++ b/sys/netlink/netlink_linux.h
@@ -38,7 +38,7 @@ struct nl_pstate;
 struct nl_writer;
 
 typedef bool msgs_to_linux_cb_t(struct nl_writer *nw, struct nlpcb *nlp);
-typedef struct nlmsghdr *msg_from_linux_cb_t(int netlink_family, struct nlmsghdr *hdr,
+typedef int msg_from_linux_cb_t(int netlink_family, struct nlmsghdr **hdr,
     struct nl_pstate *npt);
 
 struct linux_netlink_provider {