mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-31 08:15:07 -04:00
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).
nfsd_nl_listener_set_doit() also parsed and validated each listener
entry inline in two separate loops, interleaved with mutating the
running listener configuration. The validation was duplicated, used an
open-coded "nla_len < sizeof(struct sockaddr)" check that was too short
for AF_INET6, and handled a malformed entry inconsistently depending on
which loop noticed it.
Add an nfsd_nl_validate_listeners() helper that walks the entire list
once and confirms each entry parses, carries both an address and a
transport name, and is long enough for its address family
(sizeof(struct sockaddr_in) for AF_INET, sizeof(struct sockaddr_in6)
for AF_INET6, -EAFNOSUPPORT otherwise). Call it before taking
nfsd_mutex or creating the serv, so a malformed request fails cleanly
with no side effects.
Since every entry is known valid by the time the two existing loops
run, drop the redundant presence and per-family length checks from
both, leaving only the nla_parse_nested() call needed to extract the
data.
Fixes: 16a4711774 ("NFSD: add listener-{set,get} netlink command")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260615-nfsd-testing-v5-1-188d75aedda0@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
This commit is contained in:
@@ -1971,6 +1971,60 @@ int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info)
|
||||
return err;
|
||||
}
|
||||
|
||||
/**
|
||||
* nfsd_nl_validate_listeners - sanity-check the listener list from userland
|
||||
* @info: netlink metadata and command arguments
|
||||
*
|
||||
* Walk every NFSD_A_SERVER_SOCK_ADDR attribute and confirm that each entry
|
||||
* is well-formed: it parses against the policy, carries both an address and
|
||||
* a transport name, and the address is long enough for its family. Doing
|
||||
* this up front lets the callers below assume every entry is valid and
|
||||
* guarantees we make no changes when the request is malformed.
|
||||
*
|
||||
* Return: 0 if every entry is valid, or a negative errno otherwise.
|
||||
*/
|
||||
static int nfsd_nl_validate_listeners(struct genl_info *info)
|
||||
{
|
||||
const struct nlattr *attr;
|
||||
int rem;
|
||||
|
||||
nlmsg_for_each_attr_type(attr, NFSD_A_SERVER_SOCK_ADDR, info->nlhdr,
|
||||
GENL_HDRLEN, rem) {
|
||||
struct nlattr *tb[NFSD_A_SOCK_MAX + 1];
|
||||
struct sockaddr *sa;
|
||||
int err;
|
||||
|
||||
err = nla_parse_nested(tb, NFSD_A_SOCK_MAX, attr,
|
||||
nfsd_sock_nl_policy, info->extack);
|
||||
if (err < 0)
|
||||
return err;
|
||||
|
||||
if (!tb[NFSD_A_SOCK_ADDR] || !tb[NFSD_A_SOCK_TRANSPORT_NAME])
|
||||
return -EINVAL;
|
||||
|
||||
sa = nla_data(tb[NFSD_A_SOCK_ADDR]);
|
||||
if (nla_len(tb[NFSD_A_SOCK_ADDR]) < sizeof(sa->sa_family))
|
||||
return -EINVAL;
|
||||
|
||||
switch (sa->sa_family) {
|
||||
case AF_INET:
|
||||
if (nla_len(tb[NFSD_A_SOCK_ADDR]) <
|
||||
sizeof(struct sockaddr_in))
|
||||
return -EINVAL;
|
||||
break;
|
||||
case AF_INET6:
|
||||
if (nla_len(tb[NFSD_A_SOCK_ADDR]) <
|
||||
sizeof(struct sockaddr_in6))
|
||||
return -EINVAL;
|
||||
break;
|
||||
default:
|
||||
return -EAFNOSUPPORT;
|
||||
}
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* nfsd_nl_listener_set_doit - set the nfs running sockets
|
||||
* @skb: reply buffer
|
||||
@@ -1989,6 +2043,15 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
|
||||
bool delete = false;
|
||||
int err, rem;
|
||||
|
||||
/*
|
||||
* Validate the entire listener list before making any changes, so a
|
||||
* malformed request fails cleanly without creating a serv or touching
|
||||
* the existing listeners.
|
||||
*/
|
||||
err = nfsd_nl_validate_listeners(info);
|
||||
if (err)
|
||||
return err;
|
||||
|
||||
mutex_lock(&nfsd_mutex);
|
||||
|
||||
err = nfsd_create_serv(net);
|
||||
@@ -2015,16 +2078,11 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
|
||||
const char *xcl_name;
|
||||
struct sockaddr *sa;
|
||||
|
||||
/* validated up front in nfsd_nl_validate_listeners() */
|
||||
if (nla_parse_nested(tb, NFSD_A_SOCK_MAX, attr,
|
||||
nfsd_sock_nl_policy, info->extack) < 0)
|
||||
continue;
|
||||
|
||||
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]);
|
||||
|
||||
@@ -2076,16 +2134,11 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
|
||||
struct sockaddr *sa;
|
||||
int ret;
|
||||
|
||||
/* validated up front in nfsd_nl_validate_listeners() */
|
||||
if (nla_parse_nested(tb, NFSD_A_SOCK_MAX, attr,
|
||||
nfsd_sock_nl_policy, info->extack) < 0)
|
||||
continue;
|
||||
|
||||
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]);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user