@@ -1709,13 +1709,14 @@ call_ad(struct net *net, struct sock *ctnl, struct sk_buff *skb,
struct nlmsghdr *rep, *nlh = nlmsg_hdr(skb);
struct sk_buff *skb2;
struct nlmsgerr *errmsg;
- size_t payload = min(SIZE_MAX,
- sizeof(*errmsg) + nlmsg_len(nlh));
+ size_t payload;
int min_len = nlmsg_total_size(sizeof(struct nfgenmsg));
struct nlattr *cda[IPSET_ATTR_CMD_MAX + 1];
struct nlattr *cmdattr;
u32 *errline;
+ if (check_add_overflow(sizeof(*errmsg), nlmsg_len(nlh), &payload))
+ return -ENOMEM;
skb2 = nlmsg_new(payload, GFP_KERNEL);
if (!skb2)
return -ENOMEM;
@@ -1723,7 +1724,10 @@ call_ad(struct net *net, struct sock *ctnl, struct sk_buff *skb,
nlh->nlmsg_seq, NLMSG_ERROR, payload, 0);
errmsg = nlmsg_data(rep);
errmsg->error = ret;
- memcpy(&errmsg->msg, nlh, nlh->nlmsg_len);
+ unsafe_memcpy(&errmsg->msg, nlh, nlh->nlmsg_len,
+ /* "payload" was explicitly bounds-checked, based on
+ * the size of nlh->nlmsg_len.
+ */);
cmdattr = (void *)&errmsg->msg + min_len;
ret = nla_parse(cda, IPSET_ATTR_CMD_MAX, cmdattr,
@@ -2407,7 +2407,7 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
struct nlmsghdr *rep;
struct nlmsgerr *errmsg;
size_t payload = sizeof(*errmsg);
- size_t tlvlen = 0;
+ size_t alloc_size, tlvlen = 0;
struct netlink_sock *nlk = nlk_sk(NETLINK_CB(in_skb).sk);
unsigned int flags = 0;
bool nlk_has_extack = nlk->flags & NETLINK_F_EXT_ACK;
@@ -2419,32 +2419,44 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
if (nlk_has_extack && extack && extack->_msg)
tlvlen += nla_total_size(strlen(extack->_msg) + 1);
- if (err && !(nlk->flags & NETLINK_F_CAP_ACK))
- payload += nlmsg_len(nlh);
+ if (err && !(nlk->flags & NETLINK_F_CAP_ACK) &&
+ check_add_overflow(payload, (size_t)nlmsg_len(nlh), &payload))
+ goto failure;
else
flags |= NLM_F_CAPPED;
- if (err && nlk_has_extack && extack && extack->bad_attr)
- tlvlen += nla_total_size(sizeof(u32));
- if (nlk_has_extack && extack && extack->cookie_len)
- tlvlen += nla_total_size(extack->cookie_len);
- if (err && nlk_has_extack && extack && extack->policy)
- tlvlen += netlink_policy_dump_attr_size_estimate(extack->policy);
+ if (err && nlk_has_extack && extack && extack->bad_attr &&
+ check_add_overflow(tlvlen, (size_t)nla_total_size(sizeof(u32)),
+ &tlvlen))
+ goto failure;
+ if (nlk_has_extack && extack && extack->cookie_len &&
+ check_add_overflow(tlvlen, (size_t)nla_total_size(extack->cookie_len),
+ &tlvlen))
+ goto failure;
+ if (err && nlk_has_extack && extack && extack->policy &&
+ check_add_overflow(tlvlen,
+ (size_t)netlink_policy_dump_attr_size_estimate(extack->policy),
+ &tlvlen))
+ goto failure;
if (tlvlen)
flags |= NLM_F_ACK_TLVS;
- skb = nlmsg_new(payload + tlvlen, GFP_KERNEL);
- if (!skb) {
- NETLINK_CB(in_skb).sk->sk_err = ENOBUFS;
- sk_error_report(NETLINK_CB(in_skb).sk);
- return;
- }
+ if (check_add_overflow(payload, tlvlen, &alloc_size))
+ goto failure;
+
+ skb = nlmsg_new(alloc_size, GFP_KERNEL);
+ if (!skb)
+ goto failure;
rep = __nlmsg_put(skb, NETLINK_CB(in_skb).portid, nlh->nlmsg_seq,
NLMSG_ERROR, payload, flags);
errmsg = nlmsg_data(rep);
errmsg->error = err;
- memcpy(&errmsg->msg, nlh, payload > sizeof(*errmsg) ? nlh->nlmsg_len : sizeof(*nlh));
+ unsafe_memcpy(&errmsg->msg, nlh, payload > sizeof(*errmsg)
+ ? nlh->nlmsg_len : sizeof(*nlh),
+ /* "payload" was bounds checked against nlh->nlmsg_len,
+ * and overflow-checked as tlvlen was constructed.
+ */);
if (nlk_has_extack && extack) {
if (extack->_msg) {
@@ -2469,6 +2481,11 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
nlmsg_end(skb, rep);
nlmsg_unicast(in_skb->sk, skb, NETLINK_CB(in_skb).portid);
+ return;
+failure:
+ NETLINK_CB(in_skb).sk->sk_err = ENOBUFS;
+ sk_error_report(NETLINK_CB(in_skb).sk);
+
}
EXPORT_SYMBOL(netlink_ack);
For 32-bit systems, it might be possible to wrap lnmsgerr content lengths beyond SIZE_MAX. Explicitly test for all overflows, and mark the memcpy() as being unable to internally diagnose overflows. This also excludes netlink from the coming runtime bounds check on memcpy(), since it's an unusual case of open-coded sizing and allocation. Cc: Pablo Neira Ayuso <pablo@netfilter.org> Cc: Jozsef Kadlecsik <kadlec@netfilter.org> Cc: Florian Westphal <fw@strlen.de> Cc: "David S. Miller" <davem@davemloft.net> Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: syzbot <syzkaller@googlegroups.com> Cc: netfilter-devel@vger.kernel.org Cc: coreteam@netfilter.org Cc: netdev@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- net/netfilter/ipset/ip_set_core.c | 10 +++++-- net/netlink/af_netlink.c | 49 +++++++++++++++++++++---------- 2 files changed, 40 insertions(+), 19 deletions(-)