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 b19b25d184d30d4a22aff21b98f9f13da206471f..c41d034165335baf28629c36364f68b8d2edb7d7 100644 (file)
--- 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 00154197ac3b291a783b75361040b409813cfb99..6a8839553345093310f3ff308de8aed19b71209c 100644 (file)
--- 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