bitstring: exit early if _start is past size of the bitstring

bit_ffs_at and bit_ffc_at both take _start parameters which indicate to
start searching from _start onwards.

If the given _start index is past the size of the bit string, these
functions will calculate an address of the current bitstring which is
after the expected size. The function will also dereference the memory,
resulting in a read buffer overflow.

The output of the function remains correct, because the tests ensure to
stop the loop if the current bitstring chunk passes the stop bitstring
chunk, and because of a check to ensure the reported _value is never
past _nbits.

However, if <sys/bitstring.h> is ever used in code which is checked by
-fsanitize=undefined, or similar static analysis, it can produce
warnings about reading past the buffer size.

Because of the above mentioned checks, these buffer overflows do not
occur as long as _start is less than _nbits. Additionally, by definition
bit_ffs_at and bif_ffc_at should set _result to -1 in any case where the
_start is after the _nbits.

Check for this case at the start of the function and exit early if so,
preventing the buffer read overflow, and reducing the amount of
computation that occurs.

Note that it may seem odd to ever have code that could call bit_ffc_at
or bit_ffs_at with a _start value greater than _nbits. However, consider
a for-loop that used bit_ffs and bit_ffs_at to loop over a bit string
and perform some operation on each bit that was set. If the last bit of
the bit string was set, the simplest loop implementation would call
bit_ffs_at with a start of _nbits, and expect that to return -1. While
it does infact perform correctly, this is what ultimately triggers the
unexpected buffer read overflow.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Submitted by:	Jacob Keller <jacob.e.keller@intel.com>
Reviewed by:	asomers@, erj@
MFC after:	1 week
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D22398

sys/sys/bitstring.h

@@ -202,6 +202,11 @@ bit_ffs_at(bitstr_t *_bitstr, int _start, int _nbits, int *_result)
 	bitstr_t _test;
 	int _value, _offset;
 
+	if (_start >= _nbits) {
+		*_result = -1;
+		return;
+	}
+
 	if (_nbits > 0) {
 		_curbitstr = _bitstr + _bit_idx(_start);
 		_stopbitstr = _bitstr + _bit_idx(_nbits - 1);
@@ -231,6 +236,11 @@ bit_ffc_at(bitstr_t *_bitstr, int _start, int _nbits, int *_result)
 	bitstr_t _test;
 	int _value, _offset;
 
+	if (_start >= _nbits) {
+		*_result = -1;
+		return;
+	}
+
 	if (_nbits > 0) {
 		_curbitstr = _bitstr + _bit_idx(_start);
 		_stopbitstr = _bitstr + _bit_idx(_nbits - 1);

tests/sys/sys/bitstring_test.c

@@ -246,6 +246,17 @@ BITSTRING_TC_DEFINE(bit_ffs_at)
 			    nbits, memloc, i, found_set_bit);
 		}
 	}
+
+	/* Pass a start value beyond the size of the bit string */
+	bit_ffs_at(bitstr, nbits, nbits, &found_set_bit);
+	ATF_REQUIRE_MSG(found_set_bit == -1,
+			"bit_ffs_at_%d_%s: Failed with high start value of %d, Result %d",
+			nbits, memloc, nbits, found_set_bit);
+
+	bit_ffs_at(bitstr, nbits + 3, nbits, &found_set_bit);
+	ATF_REQUIRE_MSG(found_set_bit == -1,
+			"bit_ffs_at_%d_%s: Failed with high start value of %d, Result %d",
+			nbits, memloc, nbits + 3, found_set_bit);
 }
 
 BITSTRING_TC_DEFINE(bit_ffc_at)
@@ -297,6 +308,17 @@ BITSTRING_TC_DEFINE(bit_ffc_at)
 			    nbits, memloc, i, found_clear_bit);
 		}
 	}
+
+	/* Pass a start value beyond the size of the bit string */
+	bit_ffc_at(bitstr, nbits, nbits, &found_clear_bit);
+	ATF_REQUIRE_MSG(found_clear_bit == -1,
+			"bit_ffc_at_%d_%s: Failed with high start value, Result %d",
+			nbits, memloc, found_clear_bit);
+
+	bit_ffc_at(bitstr, nbits + 3, nbits, &found_clear_bit);
+	ATF_REQUIRE_MSG(found_clear_bit == -1,
+			"bit_ffc_at_%d_%s: Failed with high start value of %d, Result %d",
+			nbits, memloc, nbits + 3, found_clear_bit);
 }
 
 BITSTRING_TC_DEFINE(bit_nclear)