Commit cff2d762 authored by Jakub Kicinski's avatar Jakub Kicinski

genetlink: reject use of nlmsg_flags for new commands

Commit 9c5d03d3 ("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: default avatarJohannes Berg <johannes@sipsolutions.net>
Acked-by: default avatarNikolay Aleksandrov <razor@blackwall.org>
Reviewed-by: default avatarNicolas Dichtel <nicolas.dichtel@6wind.com>
Reviewed-by: default avatarGuillaume Nault <gnault@redhat.com>
Reviewed-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Link: https://lore.kernel.org/r/20220929142809.1167546-1-kuba@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 915b96c5
...@@ -739,6 +739,36 @@ static int genl_family_rcv_msg_doit(const struct genl_family *family, ...@@ -739,6 +739,36 @@ static int genl_family_rcv_msg_doit(const struct genl_family *family,
return err; 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, static int genl_family_rcv_msg(const struct genl_family *family,
struct sk_buff *skb, struct sk_buff *skb,
struct nlmsghdr *nlh, struct nlmsghdr *nlh,
...@@ -757,7 +787,7 @@ static int genl_family_rcv_msg(const struct genl_family *family, ...@@ -757,7 +787,7 @@ static int genl_family_rcv_msg(const struct genl_family *family,
if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen)) if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen))
return -EINVAL; return -EINVAL;
if (hdr->cmd >= family->resv_start_op && hdr->reserved) if (genl_header_check(family, nlh, hdr, extack))
return -EINVAL; return -EINVAL;
if (genl_get_cmd(hdr->cmd, family, &op)) if (genl_get_cmd(hdr->cmd, family, &op))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment