netlink: split up copies in the ack construction

Clean up the use of unsafe_memcpy() by adding a flexible array
at the end of netlink message header and splitting up the header
and data copies.

Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 4418b19..784b468 100644 (file)
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -931,6 +931,27 @@ static inline struct nlmsghdr *nlmsg_put(struct sk_buff *skb, u32 portid, u32 se
        return __nlmsg_put(skb, portid, seq, type, payload, flags);
 }
 
+/**
+ * nlmsg_append - Add more data to a nlmsg in a skb
+ * @skb: socket buffer to store message in
+ * @size: length of message payload
+ *
+ * Append data to an existing nlmsg, used when constructing a message
+ * with multiple fixed-format headers (which is rare).
+ * Returns NULL if the tailroom of the skb is insufficient to store
+ * the extra payload.
+ */
+static inline void *nlmsg_append(struct sk_buff *skb, u32 size)
+{
+       if (unlikely(skb_tailroom(skb) < NLMSG_ALIGN(size)))
+               return NULL;
+
+       if (NLMSG_ALIGN(size) - size)
+               memset(skb_tail_pointer(skb) + size, 0,
+                      NLMSG_ALIGN(size) - size);
+       return __skb_put(skb, NLMSG_ALIGN(size));
+}
+
 /**
  * nlmsg_put_answer - Add a new callback based netlink message to an skb
  * @skb: socket buffer to store message in

diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index e2ae82e..5da0da5 100644 (file)
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -48,6 +48,7 @@ struct sockaddr_nl {
  * @nlmsg_flags: Additional flags
  * @nlmsg_seq:   Sequence number
  * @nlmsg_pid:   Sending process port ID
+ * @nlmsg_data:  Message payload
  */
 struct nlmsghdr {
        __u32           nlmsg_len;
@@ -55,6 +56,7 @@ struct nlmsghdr {
        __u16           nlmsg_flags;
        __u32           nlmsg_seq;
        __u32           nlmsg_pid;
+       __u8            nlmsg_data[];
 };
 
 /* Flags values */

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index f0c94d3..b10d5e5 100644 (file)
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2499,19 +2499,24 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
                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 (!skb)
+               goto err_bad_put;
 
        rep = nlmsg_put(skb, NETLINK_CB(in_skb).portid, nlh->nlmsg_seq,
-                       NLMSG_ERROR, payload, flags);
+                       NLMSG_ERROR, sizeof(*errmsg), flags);
+       if (!rep)
+               goto err_bad_put;
        errmsg = nlmsg_data(rep);
        errmsg->error = err;
-       unsafe_memcpy(&errmsg->msg, nlh, payload > sizeof(*errmsg)
-                                        ? nlh->nlmsg_len : sizeof(*nlh),
-                     /* Bounds checked by the skb layer. */);
+       errmsg->msg = *nlh;
+
+       if (!(flags & NLM_F_CAPPED)) {
+               if (!nlmsg_append(skb, nlmsg_len(nlh)))
+                       goto err_bad_put;
+
+               memcpy(errmsg->msg.nlmsg_data, nlh->nlmsg_data,
+                      nlmsg_len(nlh));
+       }
 
        if (tlvlen)
                netlink_ack_tlv_fill(in_skb, skb, nlh, err, extack);
@@ -2519,6 +2524,12 @@ 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;
+
+err_bad_put:
+       NETLINK_CB(in_skb).sk->sk_err = ENOBUFS;
+       sk_error_report(NETLINK_CB(in_skb).sk);
 }
 EXPORT_SYMBOL(netlink_ack);