Re: [PATCH] af_netlink: give correct bounds to dump skb for NLMSG_DONE

[Johannes Berg]

On Tue, 2017-11-07 at 20:29 +0900, Jason A. Donenfeld wrote:
> 
> This patch thus reserves and restores the required length for
> NLMSG_DONE during the call to the dump function.
> 

That basically removes that space though, even when the dump isn't
complete... wouldn't it be better to do something like this?

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index f34750691c5c..fccf83598dab 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2183,9 +2183,15 @@ static int netlink_dump(struct sock *sk)
        skb_reserve(skb, skb_tailroom(skb) - alloc_size);
        netlink_skb_set_owner_r(skb, sk);
 
-       len = cb->dump(skb, cb);
+       /* if the dump is already done, just complete */
+       if (nlk->dump_done)
+               len = 0;
+       else
+               len = cb->dump(skb, cb);
+
+       nlk->dump_done = len == 0;
 
-       if (len > 0) {
+       if (len > 0 || skb_tailroom(skb) < nlmsg_total_size(sizeof(len))) {
                mutex_unlock(nlk->cb_mutex);
 
                if (sk_filter(sk, skb))
@@ -2196,7 +2202,7 @@ static int netlink_dump(struct sock *sk)
        }
 
        nlh = nlmsg_put_answer(skb, cb, NLMSG_DONE, sizeof(len), NLM_F_MULTI);
-       if (!nlh)
+       if (WARN_ON(!nlh))
                goto errout_skb;
 
        nl_dump_check_consistent(cb, nlh);
@@ -2273,6 +2279,7 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff 
*skb,
        }
 
        nlk->cb_running = true;
+       nlk->dump_done = false;
 
        mutex_unlock(nlk->cb_mutex);
 
diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h
index 3490f2430532..91a3652d384f 100644
--- a/net/netlink/af_netlink.h
+++ b/net/netlink/af_netlink.h
@@ -33,6 +33,7 @@ struct netlink_sock {
        wait_queue_head_t       wait;
        bool                    bound;
        bool                    cb_running;
+       bool                    dump_done;
        struct netlink_callback cb;
        struct mutex            *cb_mutex;
        struct mutex            cb_def_mutex;


https://p.sipsolutions.net/90574c3c0116d68a.txt

(untested)

johannes