[PATCH v3] nfsd: validate sockaddr length per family in listener_set

Jeff Layton <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.nfs
Message-ID <[email protected]>
nfsd_sock_nl_policy declares NFSD_A_SOCK_ADDR as bare NLA_BINARY
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).

Tighten the policy to NLA_POLICY_MIN_LEN(16) so that nla_parse_nested()
rejects anything shorter than a struct sockaddr, and add per-family
length validation in both nlmsg_for_each_attr_type loops to cover the
larger struct sockaddr_in6. The new policy floor subsumes the open-coded
"nla_len < sizeof(struct sockaddr)" check, so drop it from both loops.

In the listener-creation loop, report the error rather than silently
succeeding. Previously an unsupported family reached
svc_xprt_create_from_sa(), which returned -EAFNOSUPPORT to userspace;
simply skipping the malformed attribute would instead return 0. Set
-EAFNOSUPPORT for unsupported families and -EINVAL for a too-short
address before continuing, so userspace still sees the failure.

Fixes: 16a471177496 ("NFSD: add listener-{set,get} netlink command")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <[email protected]>
---
This version fixes the error handling so that an invalid address passed
from userland will properly cause a -EINVAL return.
---
 Documentation/netlink/specs/nfsd.yaml |  4 ++++
 fs/nfsd/netlink.c                     |  2 +-
 fs/nfsd/nfsctl.c                      | 41 ++++++++++++++++++++++++++++++-----
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index 8f36fadd68f7..9677ba19ffcd 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -156,6 +156,10 @@ attribute-sets:
       -
         name: addr
         type: binary
+        # 16 == sizeof(struct sockaddr_in); AF_INET6 callers
+        # validate the full sockaddr_in6 length in nfsctl.c.
+        checks:
+          min-len: 16
       -
         name: transport-name
         type: string
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index fbee3676d253..6570960034f1 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -37,7 +37,7 @@ const struct nla_policy nfsd_fslocations_nl_policy[NFSD_A_FSLOCATIONS_LOCATION +
 };
 
 const struct nla_policy nfsd_sock_nl_policy[NFSD_A_SOCK_TRANSPORT_NAME + 1] = {
-	[NFSD_A_SOCK_ADDR] = { .type = NLA_BINARY, },
+	[NFSD_A_SOCK_ADDR] = NLA_POLICY_MIN_LEN(16),
 	[NFSD_A_SOCK_TRANSPORT_NAME] = { .type = NLA_NUL_STRING, },
 };
 
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index f1ecbb13f642..64e9cdd17628 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -2022,12 +2022,24 @@ 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]);
 
+		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 +2095,29 @@ 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]);
 
+		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,
-- 
Jeff Layton <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.