4 ms·
That memcpy will read past the buffer boundary for small strings. Possibly a segfault or even information disclosure if string_to_uint64 is naively reused for a
by DenisM 4y ago
That memcpy will read past the buffer boundary for small strings. Possibly a segfault or even information disclosure if string_to_uint64 is naively reused for another purpose.
You’re probably “safe” wrt segfaults with default memory allocators since memory blocks will be padded to 8 bytes, but a more compact allocator will expose the flaw.
On a related note, I vaguely recall that monkeying with allocator padding and alignment is a great way to suss out bugs like this. E.g. allocate all requested blocks of memory directly flush against the boundary of a decommitted page, forcing a segfault on even single-byte overruns. Wastes a lot of memory and forces slow unaligned memory access, but keeps you honest when justified.
- hermitdev 4y agoIt seems to me there's a couple of other assumptions in there, too: 1) the string_view is null terminated (not required by string_view, not a safe assumption in general) and 2) that any padding is zero-initialized (don't think this is required, either).
- benmmurphy 4y agoThe padding doesn’t have to be 0 because the comparisons are masked off so the padding bytes are not included. Also, there is no null termination assumption. Though, I guess the last ‘fastest’ example does have the null terminator assumption but this should be assumed because it did assume zero padding. The only other assumption is that 8 bytes can be safely read and this is assumed in the original post. There might be an endian assumption.
- Dylan16807 4y ago> read past the buffer "if you can tell your compiler that the string you receive is ‘padded’ so that you can read eight bytes safely from it." But even if you can't make that guarantee, this still works pretty well as a performance comparison, you just need to make sure your final code has a runtime length check.
- vlovich123 4y agoAnd that your allocation isn’t at the end of a page.
- Dylan16807 4y agoIf you legitimately have the buffer length then it doesn't matter how the allocation is aligned. The extra time to read the next page with no errors doesn't need to be avoided, since it's rare and you're very likely to use that data anyway. The only thing you need is certainty that you have the buffer length.
- vlovich123 4y agoYou're assuming the next page is mapped. If it's pointing into unmapped memory, then you'll segfault.
- Dylan16807 4y agoI'm not assuming. When I say "guarantee", I mean the code is designed so the first 8 bytes are always mapped. When I say "runtime length check", I mean the code only runs if the first 8 bytes are in fact mapped. No assumptions. No possible way to segfault. The read is valid. If buffer[6] is not mapped, then you don't "legitimately have the buffer length".
- vlovich123 4y agoOk then I just misunderstood / some kind of language preciseness confusion. We’re on the same page now. AFAIK this is what the article is saying to do.
- Dylan16807 4y ago> AFAIK this is what the article is saying to do. Yeah, that's why I quoted the article.