On 9/20/24 4:12 AM, Ján Tomko wrote:
On a Thursday in 2024, Laine Stump wrote:
[...] + startHost = ntohl(startv4.val); + endHost = ntohl(endv4.val); + + if (endHost - startHost > 65535) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("range %1$s - %2$s is too large (> 65535)"), + startStr, endStr); + return -1; } - ret = (t2.bytes[2] - t1.bytes[2]) * 256 + (t2.bytes[3] - t1.bytes[3]); - if (ret < 0) { + + if (endHost < startHost) {
This check needs to happen before you subtract two unsigned integers, otherwise the substraction above overflows and this is essentially dead code.
Before this patch:
$ VIR_TEST_RANGE=48 VIR_TEST_DEBUG=1 ./run tests/sockettest TEST: sockettest 48) Test range 192.168.122.20 -> 192.168.122.1(192.168.122.1/24) size -1 ... libvirt: error : internal error: range 192.168.122.20 - 192.168.122.1 is reversed OK
After:
$ VIR_TEST_RANGE=48 VIR_TEST_DEBUG=1 ./run tests/sockettest TEST: sockettest 48) Test range 192.168.122.20 -> 192.168.122.1(192.168.122.1/24) size -1 ... libvirt: error : internal error: range 192.168.122.20 - 192.168.122.1 is too large (> 65535) OK
Ooh! Nice catch! I hadn't noticed because I was putting too much faith in the unit tests, and they had "successfully failed as expected" both before and after and I didn't look at the specific failure to notice that it was failing in a different way. I fixed it before pushing. Thanks for the reviews!