netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Phil Sutter <phil@nwl.cc>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org
Subject: [nft PATCH] monitor: Sanitize startup race condition
Date: Thu, 29 Sep 2022 00:32:48 +0200	[thread overview]
Message-ID: <20220928223248.25933-1-phil@nwl.cc> (raw)

During startup, 'nft monitor' first fetches the current ruleset and then
keeps this cache up to date based on received events. This is racey, as
any ruleset changes in between the initial fetch and the socket opening
are not recognized.

This script demonstrates the problem:

| #!/bin/bash
|
| while true; do
| 	nft flush ruleset
| 	iptables-nft -A FORWARD
| done &
| maniploop=$!
|
| trap "kill $maniploop; kill \$!; wait" EXIT
|
| while true; do
| 	nft monitor rules >/dev/null &
| 	sleep 0.2
| 	kill $!
| done

If the table add event is missed, the rule add event callback fails to
deserialize the rule and calls abort().

Avoid the inconvenient program exit by returning NULL from
netlink_delinearize_rule() instead of aborting and make callers check
the return value.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/cache.c               | 1 +
 src/monitor.c             | 5 +++++
 src/netlink_delinearize.c | 5 ++++-
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/cache.c b/src/cache.c
index f790f995e07b0..85de970f76448 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -598,6 +598,7 @@ static int list_rule_cb(struct nftnl_rule *nlr, void *data)
 
 	netlink_dump_rule(nlr, ctx);
 	rule = netlink_delinearize_rule(ctx, nlr);
+	assert(rule);
 	list_add_tail(&rule->list, &ctx->list);
 
 	return 0;
diff --git a/src/monitor.c b/src/monitor.c
index 7fa92ebfb0f3a..a6b30a18cfd25 100644
--- a/src/monitor.c
+++ b/src/monitor.c
@@ -551,6 +551,10 @@ static int netlink_events_rule_cb(const struct nlmsghdr *nlh, int type,
 
 	nlr = netlink_rule_alloc(nlh);
 	r = netlink_delinearize_rule(monh->ctx, nlr);
+	if (!r) {
+		fprintf(stderr, "W: Received event for an unknown table.\n");
+		goto out_free_nlr;
+	}
 	nlr_for_each_set(nlr, rule_map_decompose_cb, NULL,
 			 &monh->ctx->nft->cache);
 	cmd = netlink_msg2cmd(type, nlh->nlmsg_flags);
@@ -587,6 +591,7 @@ static int netlink_events_rule_cb(const struct nlmsghdr *nlh, int type,
 		break;
 	}
 	rule_free(r);
+out_free_nlr:
 	nftnl_rule_free(nlr);
 	return MNL_CB_OK;
 }
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 0da6cc78f94fa..e8b9724cbac94 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -3195,7 +3195,10 @@ struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
 	pctx->rule = rule_alloc(&netlink_location, &h);
 	pctx->table = table_cache_find(&ctx->nft->cache.table_cache,
 				       h.table.name, h.family);
-	assert(pctx->table != NULL);
+	if (!pctx->table) {
+		errno = ENOENT;
+		return NULL;
+	}
 
 	pctx->rule->comment = nftnl_rule_get_comment(nlr);
 
-- 
2.34.1


             reply	other threads:[~2022-09-28 22:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-28 22:32 Phil Sutter [this message]
2022-09-30 13:15 ` [nft PATCH] monitor: Sanitize startup race condition Pablo Neira Ayuso
2022-09-30 14:04   ` Phil Sutter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220928223248.25933-1-phil@nwl.cc \
    --to=phil@nwl.cc \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).