How big is this integer type?

This week I am on “escalation engineer” duty, which means triaging new bug reports. One of them was found by ubsan (-fsanitize=undefined) reporting an overflow in a signed integer shift. This was fun to fix because it needed a bit of lateral thinking.

treacherous shifts
resource limits
what not to do
fixed version
bonus

treacherous shifts

C’s bit shift operators require some care to avoid undefined behaviour. One of the pitfalls is that a shift is not allowed to change the sign bit in a signed integer.

It was exactly this problem that ubsan was reporting.

The problem code was in a portability wrapper around setrlimit(), and the bug depended on which platform it was compiled for. The problem code turned out to be 21 years old!

resource limits

BIND has an isc_resourcevalue_t which is always a uint64_t. The portability wrapper needs to convert it to the platform’s rlim_t. It protects against overflow in the conversion by saturating, setting the limit to its maximum value instead.

In principle, that ought to be straightforward, but we don’t know how big rlim_t is (it might be 32 bits or 64 bits) or whether it is signed or not. And there isn’t an RLIM_MAX constant for us to use to check for overflow. We can’t use RLIM_INFINITY because that might be a special out-of-range value like -1.

(POSIX requires rlim_t to be unsigned, but some unixes with long histories have kept it signed for compatibility with old code, and those platforms were where ubsan kicked up a fuss.)

what not to do

The buggy part of the old code was:

	bool rlim_t_is_signed = (((double)(rlim_t)-1) < 0);
	if (rlim_t_is_signed)
		rlim_max = ~((rlim_t)1 << (sizeof(rlim_t) * 8 - 1));

This shifts 1 into the sign bit (a no-no!) then inverts it to get the maximum positive rlim_t value.

fixed version

To avoid the problem, I needed to stay unsigned as much as possible. I can use an isc_resourcevalue_t which is unsigned and (should be!) at least as large as rlim_t. So, instead of asking how big an rlim_t is, the new question is how much bigger is an isc_resourcevalue_t?

We can count how many bytes wider our rlim_max variable is (should be 0 or 4) and whether rlim_t has a sign bit. (I couldn’t think of a better way to work that out.)

	isc_resourcevalue_t rlim_max = UINT64_MAX;
	size_t wider = sizeof(rlim_max) - sizeof(rlim_t);
	bool sign_bit = (double)(rlim_t)-1 < 0;

Then we just need to shift the excess bits out of rlim_max to get the result we want.

	rlim_max >>= CHAR_BIT * wider + sign_bit;

Then we can convert the resource limit value with overflow protection.

	rlim_value = ISC_MIN(value, rlim_max);

bonus

This change replaced 18 lines of code with 8 lines. I like patches with negative net LoC :-)