Re: [PATCH v7 18/18] fwd_rule: Fix static checkers warnings in fwd_rule_add()

6 May 2026

On Wed, May 06, 2026 at 09:46:44AM +0200, Stefano Brivio wrote:
...
On Wed, 6 May 2026 00:41:15 +1000
David Gibson  wrote:
...
On Tue, May 05, 2026 at 12:13:41PM +0200, Stefano Brivio wrote:
...
On Tue, 5 May 2026 16:22:43 +1000
David Gibson  wrote:
...
On Tue, May 05, 2026 at 01:11:42AM +0200, Stefano Brivio wrote:
...
The new checks are actually sufficient but not enough for Coverity
Scan. Now that fwd->sock_count and new->last are affected or supplied
by clients, we need explicit (albeit redundant) checks on them.
Signed-off-by: Stefano Brivio 
I'm assuming this does squash the warnings, but I think it does so in
a somewhat confusing way.
You don't need to assume that, you could try yourself without this
patch and you'll see exactly two warnings with a lot of details.
I'm getting better, but I'm by no means well yet.  Some emails were
within my capacity.  Sorting out the new license key and doing a
Coverity run, not so much.
...
...
...
---
 fwd_rule.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fwd_rule.c b/fwd_rule.c
index b55e4df..03e8e80 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -271,13 +271,22 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
      warn("Too many rules (maximum %d)", ARRAY_SIZE(fwd->rules));
      return -ENOSPC;
  }
+
  if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) {
      warn("Rules require too many listening sockets (maximum %d)",
           ARRAY_SIZE(fwd->socks));
      return -ENOSPC;
  }
+	/* Redundant, to make static checkers happy */
+	if (fwd->sock_count > ARRAY_SIZE(fwd->socks))
+		return -ENOSPC;
So there's actually two conditions that this is kind of relevant to:
1) (fwd->sock_count > ARRAY_SIZE(fwd->socks)) on entry
That means something is horribly wrong before we were even called.
So, I think that would be better as an assert().
But the (good) reason why Coverity Scan is complaining about a missing
check here is that the client is able to manipulate sock_count (via
num) in a previous call to this function,
Uh... they should never be able to make sock_count outside of [0,
ARRAY_SIZE(socks)).  If they can, that's a bug elsewhere.
...
so making us crash is exactly
what we want to avoid:
/home/sbrivio/passt/conf.c:2024:4:
  Type: Untrusted value as argument (TAINTED_SCALAR)
/home/sbrivio/passt/conf.c:1992:3: Tainted data flows to a taint sink
  1. path: Condition "read_u8(fd, &pif)", taking false branch.
/home/sbrivio/passt/conf.c:1995:3:
  2. path: Condition "pif == 0", taking false branch.
/home/sbrivio/passt/conf.c:1998:3:
  3. path: Condition "pif >= 4 /* (int)(sizeof (c->fwd_pending) / sizeof (c->fwd_pending[0])) */", taking false branch.
/home/sbrivio/passt/conf.c:1998:3:
  4. path: Condition "!(fwd = c->fwd_pending[pif])", taking false branch.
/home/sbrivio/passt/conf.c:2004:3:
  5. path: Condition "read_u32(fd, &count)", taking false branch.
/home/sbrivio/passt/conf.c:2007:3:
  6. path: Condition "count > 255U /* (1U << 8) - 1 */", taking false branch.
/home/sbrivio/passt/conf.c:2013:3:
  7. path: Condition "i < count", taking true branch.
/home/sbrivio/passt/conf.c:2014:4:
  8. tainted_argument: Calling function "fwd_rule_read" taints argument "r".
/home/sbrivio/passt/fwd_rule.c:659:2: Tainted data flows to a taint sink
  8.1. tainted_data_argument: Calling function "read_all_buf" taints parameter "*rule".
/home/sbrivio/passt/serialise.c:39:2: Tainted data flows to a taint sink
  8.1.1. var_assign_parm: Assigning: "p" = "buf".
/home/sbrivio/passt/serialise.c:41:2:
  8.1.2. path: Condition "left", taking true branch.
/home/sbrivio/passt/serialise.c:44:3:
  8.1.3. path: Condition "left <= len", taking true branch.
/home/sbrivio/passt/serialise.c:47:4:
  8.1.4. tainted_data_argument: Calling function "read" taints parameter "*p". [Note: The source code implementation of the function has been overridden by a builtin model.]
/home/sbrivio/passt/serialise.c:48:37:
  8.1.5. path: Condition "rc < 0", taking false branch.
/home/sbrivio/passt/serialise.c:50:3:
  8.1.6. path: Condition "rc < 0", taking false branch.
/home/sbrivio/passt/serialise.c:53:3:
  8.1.7. path: Condition "rc == 0", taking false branch.
/home/sbrivio/passt/serialise.c:60:2:
  8.1.8. path: Jumping back to the beginning of the loop.
/home/sbrivio/passt/serialise.c:41:2:
  8.1.9. path: Condition "left", taking true branch.
/home/sbrivio/passt/serialise.c:44:3:
  8.1.10. path: Condition "left <= len", taking true branch.
/home/sbrivio/passt/serialise.c:48:37:
  8.1.11. path: Condition "rc < 0", taking false branch.
/home/sbrivio/passt/serialise.c:50:3:
  8.1.12. path: Condition "rc < 0", taking false branch.
/home/sbrivio/passt/serialise.c:53:3:
  8.1.13. path: Condition "rc == 0", taking false branch.
/home/sbrivio/passt/serialise.c:60:2:
  8.1.14. path: Jumping back to the beginning of the loop.
/home/sbrivio/passt/serialise.c:41:2:
  8.1.15. path: Condition "left", taking false branch.
/home/sbrivio/passt/fwd_rule.c:659:2:
  8.2. path: Condition "read_all_buf(fd, rule, 40UL /* sizeof (*rule) */)", taking false branch.
/home/sbrivio/passt/conf.c:2014:4:
  9. path: Condition "fwd_rule_read(fd, &r)", taking false branch.
/home/sbrivio/passt/conf.c:2017:4:
  10. path: Condition "r.ifname[15UL /* sizeof (r.ifname) - 1 */]", taking false branch.
/home/sbrivio/passt/conf.c:2024:4:
  11. tainted_data_transitive: Call to function "fwd_rule_add" with tainted argument "r.last" transitively taints "fwd->sock_count".
/home/sbrivio/passt/fwd_rule.c:197:2: Tainted data flows to a taint sink
  11.1. path: Condition "new->first > new->last", taking false branch.
/home/sbrivio/passt/fwd_rule.c:202:2:
  11.2. path: Condition "!new->first", taking false branch.
/home/sbrivio/passt/fwd_rule.c:206:2:
  11.3. path: Condition "!new->to", taking false branch.
/home/sbrivio/passt/fwd_rule.c:206:2:
  11.4. path: Condition "(in_port_t)(new->to + new->last - new->first) < new->to", taking false branch.
/home/sbrivio/passt/fwd_rule.c:211:2:
  11.5. path: Condition "new->flags & -8 /* ~allowed_flags */", taking false branch.
/home/sbrivio/passt/fwd_rule.c:216:2:
  11.6. path: Condition "new->flags & (1UL /* 1UL << 0 */)", taking true branch.
/home/sbrivio/passt/fwd_rule.c:217:3:
  11.7. path: Condition "!inany_equals(&new->addr, (union inany_addr const *)&in6addr_any)", taking false branch.
/home/sbrivio/passt/fwd_rule.c:224:3:
  11.8. path: Condition "!(fwd->caps & (1UL /* 1UL << 0 */))", taking false branch.
/home/sbrivio/passt/fwd_rule.c:228:3:
  11.9. path: Condition "!(fwd->caps & (2UL /* 1UL << 1 */))", taking false branch.
/home/sbrivio/passt/fwd_rule.c:232:2:
  11.10. path: Falling through to end of if statement.
/home/sbrivio/passt/fwd_rule.c:242:2:
  11.11. path: Condition "new->proto == IPPROTO_TCP", taking true branch.
/home/sbrivio/passt/fwd_rule.c:243:3:
  11.12. path: Condition "!(fwd->caps & (4UL /* 1UL << 2 */))", taking false branch.
/home/sbrivio/passt/fwd_rule.c:247:2:
  11.13. path: Falling through to end of if statement.
/home/sbrivio/passt/fwd_rule.c:258:2:
  11.14. path: Condition "i < fwd->count", taking true branch.
/home/sbrivio/passt/fwd_rule.c:261:3:
  11.15. path: Condition "!fwd_rule_conflicts(new, &fwd->rules[i])", taking true branch.
/home/sbrivio/passt/fwd_rule.c:262:4:
  11.16. path: Continuing loop.
/home/sbrivio/passt/fwd_rule.c:258:2:
  11.17. path: Condition "i < fwd->count", taking true branch.
/home/sbrivio/passt/fwd_rule.c:261:3:
  11.18. path: Condition "!fwd_rule_conflicts(new, &fwd->rules[i])", taking true branch.
/home/sbrivio/passt/fwd_rule.c:262:4:
  11.19. path: Continuing loop.
/home/sbrivio/passt/fwd_rule.c:258:2:
  11.20. path: Condition "i < fwd->count", taking true branch.
/home/sbrivio/passt/fwd_rule.c:261:3:
  11.21. path: Condition "!fwd_rule_conflicts(new, &fwd->rules[i])", taking true branch.
/home/sbrivio/passt/fwd_rule.c:262:4:
  11.22. path: Continuing loop.
/home/sbrivio/passt/fwd_rule.c:258:2:
  11.23. path: Condition "i < fwd->count", taking false branch.
/home/sbrivio/passt/fwd_rule.c:270:2:
  11.24. path: Condition "fwd->count >= 255U /* (int)(sizeof (fwd->rules) / sizeof (fwd->rules[0])) */", taking false branch.
/home/sbrivio/passt/fwd_rule.c:274:2:
  11.25. path: Condition "fwd->sock_count + num > 327680U /* (int)(sizeof (fwd->socks) / sizeof (fwd->socks[0])) */", taking false branch.
/home/sbrivio/passt/fwd_rule.c:281:2:
  11.26. path: Condition "port <= new->last", taking true branch.
/home/sbrivio/passt/fwd_rule.c:282:53:
  11.27. path: Jumping back to the beginning of the loop.
/home/sbrivio/passt/fwd_rule.c:281:2:
  11.28. path: Condition "port <= new->last", taking false branch.
/home/sbrivio/passt/fwd_rule.c:285:2:
  11.29. parm_assign: Assigning: "fwd->sock_count" += "num", which taints "fwd->sock_count".
/home/sbrivio/passt/conf.c:2024:4:
  12. path: Condition "fwd_rule_add(fwd, &r) < 0", taking false branch.
/home/sbrivio/passt/conf.c:2026:3:
  13. path: Jumping back to the beginning of the loop.
/home/sbrivio/passt/conf.c:2013:3:
  14. path: Condition "i < count", taking true branch.
/home/sbrivio/passt/conf.c:2014:4:
  15. path: Condition "fwd_rule_read(fd, &r)", taking false branch.
/home/sbrivio/passt/conf.c:2017:4:
  16. path: Condition "r.ifname[15UL /* sizeof (r.ifname) - 1 */]", taking false branch.
/home/sbrivio/passt/conf.c:2024:4:
  17. tainted_data: Passing tainted expression "fwd->sock_count" to "fwd_rule_add", which uses it as an offset.
/home/sbrivio/passt/fwd_rule.c:197:2: Tainted data flows to a taint sink
  17.1. path: Condition "new->first > new->last", taking false branch.
/home/sbrivio/passt/fwd_rule.c:202:2:
  17.2. path: Condition "!new->first", taking false branch.
/home/sbrivio/passt/fwd_rule.c:206:2:
  17.3. path: Condition "!new->to", taking false branch.
/home/sbrivio/passt/fwd_rule.c:206:2:
  17.4. path: Condition "(in_port_t)(new->to + new->last - new->first) < new->to", taking false branch.
/home/sbrivio/passt/fwd_rule.c:211:2:
  17.5. path: Condition "new->flags & -8 /* ~allowed_flags */", taking false branch.
/home/sbrivio/passt/fwd_rule.c:216:2:
  17.6. path: Condition "new->flags & (1UL /* 1UL << 0 */)", taking true branch.
/home/sbrivio/passt/fwd_rule.c:217:3:
  17.7. path: Condition "!inany_equals(&new->addr, (union inany_addr const *)&in6addr_any)", taking false branch.
/home/sbrivio/passt/fwd_rule.c:224:3:
  17.8. path: Condition "!(fwd->caps & (1UL /* 1UL << 0 */))", taking false branch.
/home/sbrivio/passt/fwd_rule.c:228:3:
  17.9. path: Condition "!(fwd->caps & (2UL /* 1UL << 1 */))", taking false branch.
/home/sbrivio/passt/fwd_rule.c:232:2:
  17.10. path: Falling through to end of if statement.
/home/sbrivio/passt/fwd_rule.c:242:2:
  17.11. path: Condition "new->proto == IPPROTO_TCP", taking true branch.
/home/sbrivio/passt/fwd_rule.c:243:3:
  17.12. path: Condition "!(fwd->caps & (4UL /* 1UL << 2 */))", taking false branch.
/home/sbrivio/passt/fwd_rule.c:247:2:
  17.13. path: Falling through to end of if statement.
/home/sbrivio/passt/fwd_rule.c:258:2:
  17.14. path: Condition "i < fwd->count", taking true branch.
/home/sbrivio/passt/fwd_rule.c:261:3:
  17.15. path: Condition "!fwd_rule_conflicts(new, &fwd->rules[i])", taking true branch.
/home/sbrivio/passt/fwd_rule.c:262:4:
  17.16. path: Continuing loop.
/home/sbrivio/passt/fwd_rule.c:258:2:
  17.17. path: Condition "i < fwd->count", taking true branch.
/home/sbrivio/passt/fwd_rule.c:261:3:
  17.18. path: Condition "!fwd_rule_conflicts(new, &fwd->rules[i])", taking true branch.
/home/sbrivio/passt/fwd_rule.c:262:4:
  17.19. path: Continuing loop.
/home/sbrivio/passt/fwd_rule.c:258:2:
  17.20. path: Condition "i < fwd->count", taking true branch.
/home/sbrivio/passt/fwd_rule.c:261:3:
  17.21. path: Condition "!fwd_rule_conflicts(new, &fwd->rules[i])", taking true branch.
/home/sbrivio/passt/fwd_rule.c:262:4:
  17.22. path: Continuing loop.
/home/sbrivio/passt/fwd_rule.c:258:2:
  17.23. path: Condition "i < fwd->count", taking false branch.
/home/sbrivio/passt/fwd_rule.c:270:2:
  17.24. path: Condition "fwd->count >= 255U /* (int)(sizeof (fwd->rules) / sizeof (fwd->rules[0])) */", taking false branch.
/home/sbrivio/passt/fwd_rule.c:274:2:
  17.25. path: Condition "fwd->sock_count + num > 327680U /* (int)(sizeof (fwd->socks) / sizeof (fwd->socks[0])) */", taking false branch.
/home/sbrivio/passt/fwd_rule.c:280:2:
  17.26. data_index: Using tainted expression "fwd->sock_count" as an index to array "fwd->socks".
/home/sbrivio/passt/conf.c:2024:4:
  18. remediation: Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
At the same time, Coverity Scan doesn't realise that 'num' is positive,
Uh.. what?  num is unsigned, so it must know it's positive.
...
but we know that, so this will never trigger. As I wrote in the
comment, this check is redundant and it won't trigger. It's just good
to have to avoid noise from false positives.
Agreed, but sock_count < ARRAY_SIZE() is a fundamental invariant of
the data structure, so that makes more sense to document/enforce with
a check.
...
...
...
...
2) (fwd->sock_count + num) overflows
That's a closer-to-real concern.  I'm pretty sure we can't hit it for
real, because num is necessarily <= 65536, so as long as (1) is true
this can't overflow.  But that relies on the specific value of
ARRAY_SIZE(fwd->socks), so it's kind of fragile.
I think an explicit check for this is a good idea, but it should
actually check for this, not just side-effects of it, so:
  if (fwd->sock_count + num <= fwd->sock_count) {
      warn("Blah blah overflow");
      return -EFAULT; /* or whatever */
  }
...
fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count];
+
+	/* Redundant ('num' checked above), but not for static checkers */
+	if (new->last > ARRAY_SIZE(fwd->socks) + new->first)
+		return -ENOSPC;
This way of organising the check is very confusing to me.  I'm not
really sure what it's trying to catch.
Same as above.
I'm not sure which "above" you mean.
Same here: the warning is pretty clear:
/home/sbrivio/passt/conf.c:2024:4:
  Type: Untrusted loop bound (TAINTED_SCALAR)
/home/sbrivio/passt/conf.c:1992:3: Tainted data flows to a taint sink
  1. path: Condition "read_u8(fd, &pif)", taking false branch.
/home/sbrivio/passt/conf.c:1995:3:
  2. path: Condition "pif == 0", taking false branch.
/home/sbrivio/passt/conf.c:1998:3:
  3. path: Condition "pif >= 4 /* (int)(sizeof (c->fwd_pending) / sizeof (c->fwd_pending[0])) */", taking false branch.
/home/sbrivio/passt/conf.c:1998:3:
  4. path: Condition "!(fwd = c->fwd_pending[pif])", taking false branch.
/home/sbrivio/passt/conf.c:2004:3:
  5. path: Condition "read_u32(fd, &count)", taking false branch.
/home/sbrivio/passt/conf.c:2007:3:
  6. path: Condition "count > 255U /* (1U << 8) - 1 */", taking false branch.
/home/sbrivio/passt/conf.c:2013:3:
  7. path: Condition "i < count", taking true branch.
/home/sbrivio/passt/conf.c:2014:4:
  8. path: Condition "fwd_rule_read(fd, &r)", taking false branch.
/home/sbrivio/passt/conf.c:2017:4:
  9. path: Condition "r.ifname[15UL /* sizeof (r.ifname) - 1 */]", taking false branch.
/home/sbrivio/passt/conf.c:2024:4:
  10. path: Condition "fwd_rule_add(fwd, &r) < 0", taking false branch.
/home/sbrivio/passt/conf.c:2026:3:
  11. path: Jumping back to the beginning of the loop.
/home/sbrivio/passt/conf.c:2013:3:
  12. path: Condition "i < count", taking true branch.
/home/sbrivio/passt/conf.c:2014:4:
  13. tainted_argument: Calling function "fwd_rule_read" taints argument "r".
/home/sbrivio/passt/fwd_rule.c:659:2: Tainted data flows to a taint sink
  13.1. tainted_data_argument: Calling function "read_all_buf" taints parameter "*rule".
/home/sbrivio/passt/serialise.c:39:2: Tainted data flows to a taint sink
  13.1.1. var_assign_parm: Assigning: "p" = "buf".
/home/sbrivio/passt/serialise.c:41:2:
  13.1.2. path: Condition "left", taking true branch.
/home/sbrivio/passt/serialise.c:44:3:
  13.1.3. path: Condition "left <= len", taking true branch.
/home/sbrivio/passt/serialise.c:47:4:
  13.1.4. tainted_data_argument: Calling function "read" taints parameter "*p". [Note: The source code implementation of the function has been overridden by a builtin model.]
/home/sbrivio/passt/serialise.c:48:37:
  13.1.5. path: Condition "rc < 0", taking false branch.
/home/sbrivio/passt/serialise.c:50:3:
  13.1.6. path: Condition "rc < 0", taking false branch.
/home/sbrivio/passt/serialise.c:53:3:
  13.1.7. path: Condition "rc == 0", taking false branch.
/home/sbrivio/passt/serialise.c:60:2:
  13.1.8. path: Jumping back to the beginning of the loop.
/home/sbrivio/passt/serialise.c:41:2:
  13.1.9. path: Condition "left", taking true branch.
/home/sbrivio/passt/serialise.c:44:3:
  13.1.10. path: Condition "left <= len", taking true branch.
/home/sbrivio/passt/serialise.c:48:37:
  13.1.11. path: Condition "rc < 0", taking false branch.
/home/sbrivio/passt/serialise.c:50:3:
  13.1.12. path: Condition "rc < 0", taking false branch.
/home/sbrivio/passt/serialise.c:53:3:
  13.1.13. path: Condition "rc == 0", taking false branch.
/home/sbrivio/passt/serialise.c:60:2:
  13.1.14. path: Jumping back to the beginning of the loop.
/home/sbrivio/passt/serialise.c:41:2:
  13.1.15. path: Condition "left", taking false branch.
/home/sbrivio/passt/fwd_rule.c:659:2:
  13.2. path: Condition "read_all_buf(fd, rule, 40UL /* sizeof (*rule) */)", taking false branch.
/home/sbrivio/passt/conf.c:2014:4:
  14. path: Condition "fwd_rule_read(fd, &r)", taking false branch.
/home/sbrivio/passt/conf.c:2017:4:
  15. path: Condition "r.ifname[15UL /* sizeof (r.ifname) - 1 */]", taking false branch.
/home/sbrivio/passt/conf.c:2024:4:
  16. tainted_data: Passing tainted expression "r.last" to "fwd_rule_add", which uses it as a loop boundary.
/home/sbrivio/passt/fwd_rule.c:197:2: Tainted data flows to a taint sink
  16.1. lower_bounds: Casting narrower unsigned "new->last" to wider signed type "int" effectively tests its lower bound.
/home/sbrivio/passt/fwd_rule.c:197:2:
  16.2. path: Condition "new->first > new->last", taking false branch.
/home/sbrivio/passt/fwd_rule.c:197:2:
  16.3. lower_bounds: Checking lower bounds of unsigned scalar "new->last" by taking the false branch of "new->first > new->last".
/home/sbrivio/passt/fwd_rule.c:202:2:
  16.4. path: Condition "!new->first", taking false branch.
/home/sbrivio/passt/fwd_rule.c:206:2:
  16.5. path: Condition "!new->to", taking false branch.
/home/sbrivio/passt/fwd_rule.c:206:2:
  16.6. lower_bounds: Casting narrower unsigned "new->last" to wider signed type "int" effectively tests its lower bound.
/home/sbrivio/passt/fwd_rule.c:206:2:
  16.7. path: Condition "(in_port_t)(new->to + new->last - new->first) < new->to", taking false branch.
/home/sbrivio/passt/fwd_rule.c:211:2:
  16.8. path: Condition "new->flags & -8 /* ~allowed_flags */", taking false branch.
/home/sbrivio/passt/fwd_rule.c:216:2:
  16.9. path: Condition "new->flags & (1UL /* 1UL << 0 */)", taking true branch.
/home/sbrivio/passt/fwd_rule.c:217:3:
  16.10. path: Condition "!inany_equals(&new->addr, (union inany_addr const *)&in6addr_any)", taking false branch.
/home/sbrivio/passt/fwd_rule.c:224:3:
  16.11. path: Condition "!(fwd->caps & (1UL /* 1UL << 0 */))", taking false branch.
/home/sbrivio/passt/fwd_rule.c:228:3:
  16.12. path: Condition "!(fwd->caps & (2UL /* 1UL << 1 */))", taking false branch.
/home/sbrivio/passt/fwd_rule.c:232:2:
  16.13. path: Falling through to end of if statement.
/home/sbrivio/passt/fwd_rule.c:242:2:
  16.14. path: Condition "new->proto == IPPROTO_TCP", taking true branch.
/home/sbrivio/passt/fwd_rule.c:243:3:
  16.15. path: Condition "!(fwd->caps & (4UL /* 1UL << 2 */))", taking false branch.
/home/sbrivio/passt/fwd_rule.c:247:2:
  16.16. path: Falling through to end of if statement.
/home/sbrivio/passt/fwd_rule.c:258:2:
  16.17. path: Condition "i < fwd->count", taking true branch.
/home/sbrivio/passt/fwd_rule.c:261:3:
  16.18. path: Condition "!fwd_rule_conflicts(new, &fwd->rules[i])", taking true branch.
/home/sbrivio/passt/fwd_rule.c:262:4:
  16.19. path: Continuing loop.
/home/sbrivio/passt/fwd_rule.c:258:2:
  16.20. path: Condition "i < fwd->count", taking true branch.
/home/sbrivio/passt/fwd_rule.c:261:3:
  16.21. path: Condition "!fwd_rule_conflicts(new, &fwd->rules[i])", taking true branch.
/home/sbrivio/passt/fwd_rule.c:262:4:
  16.22. path: Continuing loop.
/home/sbrivio/passt/fwd_rule.c:258:2:
  16.23. path: Condition "i < fwd->count", taking false branch.
/home/sbrivio/passt/fwd_rule.c:270:2:
  16.24. path: Condition "fwd->count >= 255U /* (int)(sizeof (fwd->rules) / sizeof (fwd->rules[0])) */", taking false branch.
/home/sbrivio/passt/fwd_rule.c:274:2:
  16.25. path: Condition "fwd->sock_count + num > 327680U /* (int)(sizeof (fwd->socks) / sizeof (fwd->socks[0])) */", taking false branch.
/home/sbrivio/passt/fwd_rule.c:281:2:
  16.26. loop_bound_upper: Using tainted expression "new->last" as a loop boundary.
/home/sbrivio/passt/conf.c:2024:4:
  17. remediation: Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
I'll expand the comment and remove the newline between this and the for
loop.
...
...
...
We've already checked that
last >= first, so using num is safer to deal with at this
point than ARRAY_SIZE() + first, which could in principle overflow
even if sock_count + num is perfectly ok.
Using 'num' won't work. It shouldn't overflow anyway because the
addition happens in 'int'.
It shouldn't overflow, but proving that requires knowing that:
  a. sock_count is bounded by ARRAY_SIZE(socks)
  b. first, last and num are bounded by 2^16
  c. ARRAY_SIZE(socks) + 2^16 won't overflow (in unsigned int)
I'm not sure which part coverity is missing.  (c) at least requires
knowledge which is not found in immediately adjacent code.
It's missing the fact that 'num' is already checked above (that is,
a.), and due to that we know we're not overflowing fwd->rulesocks[x].
...
Oh... and... I just remembered that ARRAY_SIZE() is int, not unsigned
(or size_t).  I thought about changing that at some point, but it
seemed to cause more trouble than it was worth.  Does keep tripping me
though, since it seems like it logically ought to be unsigned.  Signed
overflows are much nastier (UB) that unsigned overflows.
...
I'll try to change the rest if I find some time but it doesn't really
look that critical to me.
...
...
for (port = new->first; port <= new->last; port++)
      fwd->rulesocks[fwd->count][port - new->first] = -1;
-- 
2.43.0
-- 
Stefano
-- 
Stefano
-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

    