netdev - [PATCH net-next RESEND] genetlink: reject use of nlmsg

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-next>] [day] [month] [year] [list]

Date:   Thu, 29 Sep 2022 07:28:09 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     davem@...emloft.net
Cc:     netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com,
        Jakub Kicinski <kuba@...nel.org>,
        Johannes Berg <johannes@...solutions.net>,
        Nikolay Aleksandrov <razor@...ckwall.org>,
        Nicolas Dichtel <nicolas.dichtel@...nd.com>,
        Guillaume Nault <gnault@...hat.com>,
        Florent Fourcot <florent.fourcot@...irst.fr>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Florian Westphal <fw@...len.de>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jacob Keller <jacob.e.keller@...el.com>,
        Hangbin Liu <liuhangbin@...il.com>
Subject: [PATCH net-next RESEND] genetlink: reject use of nlmsg_flags for new commands

Commit 9c5d03d36251 ("genetlink: start to validate reserved header bytes")
introduced extra validation for genetlink headers. We had to gate it
to only apply to new commands, to maintain bug-wards compatibility.
Use this opportunity (before the new checks make it to Linus's tree)
to add more conditions.

Validate that Generic Netlink families do not use nlmsg_flags outside
of the well-understood set.

Link: https://lore.kernel.org/all/20220928073709.1b93b74a@kernel.org/
Reviewed-by: Johannes Berg <johannes@...solutions.net>
Acked-by: Nikolay Aleksandrov <razor@...ckwall.org>
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@...nd.com>
Reviewed-by: Guillaume Nault <gnault@...hat.com>
Signed-off-by: Jakub Kicinski <kuba@...nel.org>
--
RESEND with the right tree in the subject

CC: Florent Fourcot <florent.fourcot@...irst.fr>
CC: Pablo Neira Ayuso <pablo@...filter.org>
CC: Florian Westphal <fw@...len.de>
CC: Jamal Hadi Salim <jhs@...atatu.com>
CC: Jacob Keller <jacob.e.keller@...el.com>
CC: Hangbin Liu <liuhangbin@...il.com>
---
 net/netlink/genetlink.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 7c136de117eb..39b7c00e4cef 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -739,6 +739,36 @@ static int genl_family_rcv_msg_doit(const struct genl_family *family,
 	return err;
 }
 
+static int genl_header_check(const struct genl_family *family,
+			     struct nlmsghdr *nlh, struct genlmsghdr *hdr,
+			     struct netlink_ext_ack *extack)
+{
+	u16 flags;
+
+	/* Only for commands added after we started validating */
+	if (hdr->cmd < family->resv_start_op)
+		return 0;
+
+	if (hdr->reserved) {
+		NL_SET_ERR_MSG(extack, "genlmsghdr.reserved field is not 0");
+		return -EINVAL;
+	}
+
+	/* Old netlink flags have pretty loose semantics, allow only the flags
+	 * consumed by the core where we can enforce the meaning.
+	 */
+	flags = nlh->nlmsg_flags;
+	if ((flags & NLM_F_DUMP) == NLM_F_DUMP) /* DUMP is 2 bits */
+		flags &= ~NLM_F_DUMP;
+	if (flags & ~(NLM_F_REQUEST | NLM_F_ACK | NLM_F_ECHO)) {
+		NL_SET_ERR_MSG(extack,
+			       "ambiguous or reserved bits set in nlmsg_flags");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int genl_family_rcv_msg(const struct genl_family *family,
 			       struct sk_buff *skb,
 			       struct nlmsghdr *nlh,
@@ -757,7 +787,7 @@ static int genl_family_rcv_msg(const struct genl_family *family,
 	if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen))
 		return -EINVAL;
 
-	if (hdr->cmd >= family->resv_start_op && hdr->reserved)
+	if (genl_header_check(family, nlh, hdr, extack))
 		return -EINVAL;
 
 	if (genl_get_cmd(hdr->cmd, family, &op))
-- 
2.37.3