Re: [PATCH v2 15/21] nfsd: validate sockaddr length per family in listener_set

[Jeff Layton]

On Thu, 2026-06-11 at 16:00 -0400, Jeff Layton wrote:
> 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) and add per-family
> length validation in both nlmsg_for_each_attr_type loops.
> 
> Fixes: 16a471177496 ("NFSD: add listener-{set,get} netlink command")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
>  Documentation/netlink/specs/nfsd.yaml |  4 ++++
>  fs/nfsd/netlink.c                     |  2 +-
>  fs/nfsd/nfsctl.c                      | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
> 
> 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 ab10692ee937..f3b3154b16c5 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -2016,6 +2016,21 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
>  		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 */
> @@ -2077,6 +2092,21 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
>  		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;
> +		}
> +

Sashiko flagged a problem above that we should probably fix. If we get
an invalid address here, we should return an error instead of ignoring
it. I'm working on an updated patch now.

>  		xprt = svc_find_listener(serv, xcl_name, net, sa);
>  		if (xprt) {
>  			if (delete)

-- 
Jeff Layton <jlayton@xxxxxxxxxx>