From 8fe984c7cce8c7f2e1c36b1d00abe7d092e4c0d1 Mon Sep 17 00:00:00 2001
From: Remi Pommarel <rpommarel@freebox.fr>
Date: Wed, 13 Jul 2022 11:48:55 +0200
Subject: [PATCH 7/7] Fix race in nl_cache_resync & nl_cache_refill

When doing a full cache resynchronization (in case of error), libnl3
mark all currently present object request a full dump clear the mark of
all object received in the dump and sends a del event for any object
that still has the mark at the end. Unfortunately after requestion a
full dump, libnl wait for the next received message and sends del event
for any mark object just after treating it. This message could very well
be another message than the full dump response (an event sends between
last read and dump request) causing all present object to still be
marked and thus be deleted.

The same problem applies for nl_cache_resync, so lets fix it the same way
by calling directly __cache_pickup and waiting for the actual kernel dump resp
before returning. This avoids returning a partially init cache (based on a
single async event), and reading the actual response from the kernel later on.
(which by the way can be interrupted)
---
 lib/cache.c | 31 ++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/lib/cache.c b/lib/cache.c
index eadce57..3f08349 100644
--- a/lib/cache.c
+++ b/lib/cache.c
@@ -655,8 +655,30 @@ static int nl_cache_request_full_dump(struct nl_sock *sk,
 struct update_xdata {
 	struct nl_cache_ops *ops;
 	struct nl_parser_param *params;
+	unsigned int seq_wait;
 };
 
+#define seq_before(s1, s2) (((long)s1 - (long)s2) < 0)
+
+static int update_seq_checker(struct nl_msg *msg, void *arg)
+{
+	struct nlmsghdr *hdr = nlmsg_hdr(msg);
+	unsigned int seq = hdr->nlmsg_seq;
+	struct update_xdata *x = arg;
+
+	/* Wait for last multipart message */
+	if ((hdr->nlmsg_flags & NLM_F_MULTI) && (hdr->nlmsg_type != NLMSG_DONE))
+		goto out;
+
+	if ((x->seq_wait == NL_AUTO_SEQ) || seq_before(seq, x->seq_wait))
+		goto out;
+
+	x->seq_wait = NL_AUTO_SEQ;
+
+out:
+	return 0;
+}
+
 static int update_msg_parser(struct nl_msg *msg, void *arg)
 {
 	struct update_xdata *x = arg;
@@ -677,13 +699,14 @@ static int update_msg_parser(struct nl_msg *msg, void *arg)
  * @arg param		Parser parameters
  */
 static int __cache_pickup(struct nl_sock *sk, struct nl_cache *cache,
-			  struct nl_parser_param *param)
+			  struct nl_parser_param *param, bool resync)
 {
 	int err;
 	struct nl_cb *cb;
 	struct update_xdata x = {
 		.ops = cache->c_ops,
 		.params = param,
+		.seq_wait = resync ? sk->s_seq_next - 1 : NL_AUTO_SEQ,
 	};
 
 	NL_DBG(2, "Picking up answer for cache %p <%s>\n",
@@ -694,8 +717,13 @@ static int __cache_pickup(struct nl_sock *sk, struct nl_cache *cache,
 		return -NLE_NOMEM;
 
 	nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, update_msg_parser, &x);
+	if (resync)
+		nl_cb_set(cb, NL_CB_SEQ_CHECK, NL_CB_CUSTOM, update_seq_checker, &x);
+
+	do {
+		err = nl_recvmsgs(sk, cb);
+	} while ((err >= 0) && (x.seq_wait != NL_AUTO_SEQ));
 
-	err = nl_recvmsgs(sk, cb);
 	if (err < 0)
 		NL_DBG(2, "While picking up for %p <%s>, recvmsgs() returned %d: %s\n",
 		       cache, nl_cache_name(cache), err, nl_geterror(err));
@@ -742,7 +770,7 @@ static int __nl_cache_pickup(struct nl_sock *sk, struct nl_cache *cache,
 	if (sk->s_proto != cache->c_ops->co_protocol)
 		return -NLE_PROTO_MISMATCH;
 
-	return __cache_pickup(sk, cache, &p);
+	return __cache_pickup(sk, cache, &p, false);
 }
 
 /**
@@ -940,7 +968,7 @@ restart:
 		if (err < 0)
 			goto errout;
 
-		err = __cache_pickup(sk, cache, &p);
+		err = __cache_pickup(sk, cache, &p, true);
 		if (err == -NLE_DUMP_INTR)
 			goto restart;
 		else if (err < 0)
@@ -1035,6 +1063,10 @@ int nl_cache_parse_and_add(struct nl_cache *cache, struct nl_msg *msg)
 int nl_cache_refill(struct nl_sock *sk, struct nl_cache *cache)
 {
 	struct nl_af_group *grp;
+	struct nl_parser_param p = {
+		.pp_cb = pickup_cb,
+		.pp_arg = cache,
+	};
 	int err;
 
 	if (sk->s_proto != cache->c_ops->co_protocol)
@@ -1055,7 +1087,7 @@ restart:
 		NL_DBG(2, "Updating cache %p <%s> for family %u, request sent, waiting for reply\n",
 		       cache, nl_cache_name(cache), grp ? grp->ag_family : AF_UNSPEC);
 
-		err = nl_cache_pickup(sk, cache);
+		err = __cache_pickup(sk, cache, &p, true);
 		if (err == -NLE_DUMP_INTR) {
 			NL_DBG(2, "Dump interrupted, restarting!\n");
 			goto restart;
-- 
2.37.1