Skip to contentSkip to conversation

nfsd: validate sockaddr length per family in listener_set

PATCH Superseded Jeff Layton 1 participant 1 message Jun 15

Subsystems (2)

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Jeff Layton

author maintainer
#0 lore raw

PATCH v4 1/1 nfsd: validate sockaddr length per family in listener_set

nfsd_sock_nl_policy declares NFSD_A_SOCK_ADDR as a bare NLA_BINARY attribute with no minimum length. A CAP_NET_ADMIN caller can send a 16-byte NFSD_A_SOCK_ADDR with sa_family=AF_INET6, causing a 12-byte OOB read across three consumers (rpc_cmp_addr_port, svc_find_listener, kernel_bind).

Validate the attribute length in both nlmsg_for_each_attr_type loops in nfsd_nl_listener_set_doit(): first ensure it is large enough to read sa_family, then require the full length for the address family (sizeof(struct sockaddr_in) for AF_INET, sizeof(struct sockaddr_in6) for AF_INET6), and reject any other family. This replaces the open-coded "nla_len < sizeof(struct sockaddr)" check, which was too short for AF_INET6.

In the listener-creation loop, report the error rather than silently succeeding. Previously a malformed or unsupported address was skipped, and if it was the only one supplied the call returned 0. Set -EINVAL for a too-short address and -EAFNOSUPPORT for an unsupported family before continuing, so userspace still sees the failure.

Fixes 16a471177496 ("NFSD: add listener-{set,get} netlink command")
Assisted-by Claude:claude-opus-4-8

---
Changes in v4:
- Drop policy floor and do full inline validation
- Link to v3: https://lore.kernel.org/r/[email protected]

---
 fs/nfsd/nfsctl.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 6 deletions(-)
fs/nfsd/nfsctl.c +43 -6
index f1ecbb13f642..069f37a3533e 100644
@@ -2022,12 +2022,27 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
 		if (!tb[NFSD_A_SOCK_ADDR] || !tb[NFSD_A_SOCK_TRANSPORT_NAME])
 			continue;
 
-		if (nla_len(tb[NFSD_A_SOCK_ADDR]) < sizeof(*sa))
-			continue;
-
 		xcl_name = nla_data(tb[NFSD_A_SOCK_TRANSPORT_NAME]);
 		sa = nla_data(tb[NFSD_A_SOCK_ADDR]);
 
+		if (nla_len(tb[NFSD_A_SOCK_ADDR]) < sizeof(sa->sa_family))
+			continue;
+
+		switch (sa->sa_family) {
+		case AF_INET:
+			if (nla_len(tb[NFSD_A_SOCK_ADDR]) <
+			    sizeof(struct sockaddr_in))
+				continue;
+			break;
+		case AF_INET6:
+			if (nla_len(tb[NFSD_A_SOCK_ADDR]) <
+			    sizeof(struct sockaddr_in6))
+				continue;
+			break;
+		default:
+			continue;
+		}
+
 		/* Put back any matching sockets */
 		list_for_each_entry_safe(xprt, tmp, &permsocks, xpt_list) {
 			/* This shouldn't be possible */
@@ -2083,12 +2098,34 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
 		if (!tb[NFSD_A_SOCK_ADDR] || !tb[NFSD_A_SOCK_TRANSPORT_NAME])
 			continue;
 
-		if (nla_len(tb[NFSD_A_SOCK_ADDR]) < sizeof(*sa))
-			continue;
-
 		xcl_name = nla_data(tb[NFSD_A_SOCK_TRANSPORT_NAME]);
 		sa = nla_data(tb[NFSD_A_SOCK_ADDR]);
 
+		if (nla_len(tb[NFSD_A_SOCK_ADDR]) < sizeof(sa->sa_family)) {
+			err = -EINVAL;
+			continue;
+		}
+
+		switch (sa->sa_family) {
+		case AF_INET:
+			if (nla_len(tb[NFSD_A_SOCK_ADDR]) <
+			    sizeof(struct sockaddr_in)) {
+				err = -EINVAL;
+				continue;
+			}
+			break;
+		case AF_INET6:
+			if (nla_len(tb[NFSD_A_SOCK_ADDR]) <
+			    sizeof(struct sockaddr_in6)) {
+				err = -EINVAL;
+				continue;
+			}
+			break;
+		default:
+			err = -EAFNOSUPPORT;
+			continue;
+		}
+
 		xprt = svc_find_listener(serv, xcl_name, net, sa);
 		if (xprt) {
 			if (delete)

---
base-commit: 332e2f4f37b213f231be1ab5ddc17e2052383b60
change-id: 20260608-nfsd-testing-688a82433c50

Best regards,

Signature · 1 line
Jeff Layton <[email protected]>