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 3/4] segtree: Fix for potential NULL-pointer deref in ei_insert()
Date: Mon, 20 Jan 2020 17:25:39 +0100	[thread overview]
Message-ID: <20200120162540.9699-4-phil@nwl.cc> (raw)
In-Reply-To: <20200120162540.9699-1-phil@nwl.cc>

Covscan complained about potential deref of NULL 'lei' pointer,
Interestingly this can't happen as the relevant goto leading to that
(in line 260) sits in code checking conflicts between new intervals and
since those are sorted upon insertion, only the lower boundary may
conflict (or both, but that's covered before).

Given the needed investigation to proof covscan wrong and the actually
wrong (but impossible) code, better fix this as if element ordering was
arbitrary to avoid surprises if at some point it really becomes that.

Fixes: 4d6ad0f310d6c ("segtree: check for overlapping elements at insertion")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/segtree.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/segtree.c b/src/segtree.c
index e8e32412f3a41..04c0e915263b9 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -205,8 +205,11 @@ static int ei_insert(struct list_head *msgs, struct seg_tree *tree,
 		pr_gmp_debug("insert: [%Zx %Zx]\n", new->left, new->right);
 
 	if (lei != NULL && rei != NULL && lei == rei) {
-		if (!merge)
+		if (!merge) {
+			expr_binary_error(msgs, lei->expr, new->expr,
+					  "conflicting intervals specified");
 			goto err;
+		}
 		/*
 		 * The new interval is entirely contained in the same interval,
 		 * split it into two parts:
@@ -228,8 +231,11 @@ static int ei_insert(struct list_head *msgs, struct seg_tree *tree,
 		ei_destroy(lei);
 	} else {
 		if (lei != NULL) {
-			if (!merge)
+			if (!merge) {
+				expr_binary_error(msgs, lei->expr, new->expr,
+						  "conflicting intervals specified");
 				goto err;
+			}
 			/*
 			 * Left endpoint is within lei, adjust it so we have:
 			 *
@@ -248,8 +254,11 @@ static int ei_insert(struct list_head *msgs, struct seg_tree *tree,
 			}
 		}
 		if (rei != NULL) {
-			if (!merge)
+			if (!merge) {
+				expr_binary_error(msgs, rei->expr, new->expr,
+						  "conflicting intervals specified");
 				goto err;
+			}
 			/*
 			 * Right endpoint is within rei, adjust it so we have:
 			 *
@@ -276,8 +285,7 @@ static int ei_insert(struct list_head *msgs, struct seg_tree *tree,
 	return 0;
 err:
 	errno = EEXIST;
-	return expr_binary_error(msgs, lei->expr, new->expr,
-				 "conflicting intervals specified");
+	return -1;
 }
 
 /*
-- 
2.24.1


  parent reply	other threads:[~2020-01-20 16:26 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-20 16:25 [nft PATCH 0/4] Fixes for a recent covscan run Phil Sutter
2020-01-20 16:25 ` [nft PATCH 1/4] netlink: Fix leak in unterminated string deserializer Phil Sutter
2020-01-21 12:55   ` Pablo Neira Ayuso
2020-01-20 16:25 ` [nft PATCH 2/4] netlink: Fix leaks in netlink_parse_cmp() Phil Sutter
2020-01-21 12:55   ` Pablo Neira Ayuso
2020-01-20 16:25 ` Phil Sutter [this message]
2020-01-21 12:56   ` [nft PATCH 3/4] segtree: Fix for potential NULL-pointer deref in ei_insert() Pablo Neira Ayuso
2020-01-27 13:24     ` Phil Sutter
2020-02-21 22:07       ` Phil Sutter
2020-01-20 16:25 ` [nft PATCH 4/4] netlink: Avoid potential NULL-pointer deref in netlink_gen_payload_stmt() Phil Sutter
2020-01-21 12:57   ` Pablo Neira Ayuso

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=20200120162540.9699-4-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).