Patch to further harden glibc malloc metadata against 1-byte overflows
DJ Delorie
dj@redhat.com
Thu Mar 16 20:42:00 GMT 2017
I'll review this in three parts: correctness, performance, and nitpicks.
Correctness: I think this patch is correct. All uses of the unlink()
macro are for the regular bins (which includes unsorted, but that's OK);
fastbins are handled separately, as are the sentinels and top chunks.
Since next_chunk->prev_size is part of the current chunk, it's always
available to check. This patch doesn't cover removing chunks from
fastbins, but that doesn't reduce the correctness of this patch.
Performance: I was unable to detect any significant performance
degradation due to the extra code, although obviously "more checking"
means "slower" to a certain extent. This code is generally limited to
the slower paths through malloc anyway, and I think any performance
issues are too small to worry about.
Nitpicks: the patch doesn't apply as-is, at least in the recent tree I
tested against, as prev_size is not the current field name. Also,
please use the prev_size() macro instead of a direct access. I would
like to see "in unlink" in the crash text, but that would make the
message inconsistent with the other ones. At least the message is
unique within malloc ;-) Lastly, please make sure the trailing
backslashes all line up when using GNU-standard tabstops.
Overall, I think this patch is acceptable and should be applied, with
the nitpicks fixed, of course. If you can't commit it yourself, please
post an updated patch (cc me) and I'll apply it for you.
Thanks!
DJ
More information about the Libc-alpha
mailing list