From: Guy Harris <guy@alum.mit.edu>
Date: Fri, 26 Oct 2018 20:14:37 +0000 (-0700)
Subject: Handle negation in a way that doesn't upset compilers or UBSan.
X-Git-Tag: libpcap-1.10-bp~748
X-Git-Url: https://git.tcpdump.org/libpcap/commitdiff_plain/52a246fb5291e97ba97bc07fc66e0d598484e471

Handle negation in a way that doesn't upset compilers or UBSan.

Most BPF arithmetic is unsigned, but negation can't be unsigned;
respecify it as subtracting the value from 0U, so that 1) we don't get
compiler warnings about negating an unsigned value and 2) don't get
UBSan warnings about the result of negating 0x80000000 being undefined.

Credit to OSS-Fuzz for finding these issues.
---

diff --git a/bpf_filter.c b/bpf_filter.c
index b19b25d1..c41d0341 100644
--- a/bpf_filter.c
+++ b/bpf_filter.c
@@ -358,10 +358,13 @@ pcap_filter_with_aux_data(const struct bpf_insn *pc, const u_char *p,
 		case BPF_ALU|BPF_NEG:
 			/*
 			 * Most BPF arithmetic is unsigned, but negation
-			 * can't be unsigned; throw some casts to
-			 * specify what we're trying to do.
+			 * can't be unsigned; respecify it as subtracting
+			 * the accumulator from 0U, so that 1) we don't
+			 * get compiler warnings about negating an unsigned
+			 * value and 2) don't get UBSan warnings about
+			 * the result of negating 0x80000000 being undefined.
 			 */
-			A = (uint32_t)(-(int32_t)A);
+			A = (0U - A);
 			continue;
 
 		case BPF_MISC|BPF_TAX:
diff --git a/optimize.c b/optimize.c
index 00154197..6a883955 100644
--- a/optimize.c
+++ b/optimize.c
@@ -1142,7 +1142,23 @@ opt_stmt(opt_state_t *opt_state, struct stmt *s, int val[], int alter)
 	case BPF_ALU|BPF_NEG:
 		if (alter && opt_state->vmap[val[A_ATOM]].is_const) {
 			s->code = BPF_LD|BPF_IMM;
-			s->k = -opt_state->vmap[val[A_ATOM]].const_val;
+			/*
+			 * Do this negation as unsigned arithmetic; that's
+			 * what modern BPF engines do, and it guarantees
+			 * that all possible values can be negated.  (Yeah,
+			 * negating 0x80000000, the minimum signed 32-bit
+			 * two's-complement value, results in 0x80000000,
+			 * so it's still negative, but we *should* be doing
+			 * all unsigned arithmetic here, to match what
+			 * modern BPF engines do.)
+			 *
+			 * Express it as 0U - (unsigned value) so that we
+			 * don't get compiler warnings about negating an
+			 * unsigned value and don't get UBSan warnings
+			 * about the result of negating 0x80000000 being
+			 * undefined.
+			 */
+			s->k = 0U - (bpf_u_int32)(opt_state->vmap[val[A_ATOM]].const_val);
 			val[A_ATOM] = K(s->k);
 		}
 		else