Page MenuHomeFreeBSD

CTF: Fix an out-of-bounds read
ClosedPublic

Authored by domagoj.stolfa_gmail.com on Fri, Mar 26, 7:18 PM.

Details

Summary

When prefixes such as struct, union, etc. are compared with the current type (e.g. struct foo), a comparison is made with the prefix. The code currently assumes that every type is a valid C type with a prefix, however at times, garbage ends up in this function causing an unpredictable crash with DTrace due to the isspace(*p) call or subsequent calls. An example that I've seen of this is the letter 's' being passed in, comparing true with struct as the comparison size was (q - p) == 1, but then we increment p with the length of "struct", resulting in an out of bounds read. This diff simply adds some robustness checks to the surrounding code to prevent this from happening and causing weird behaviour.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Thanks, this looks right to me.

This revision is now accepted and ready to land.Sat, Mar 27, 2:50 PM

(Please let me know if you'd like me to commit this.)

(Please let me know if you'd like me to commit this.)

I think this is fine to commit -- I've been running it and ran into no issues. Thanks!

cddl/contrib/opensolaris/common/ctf/ctf_lookup.c
94

Sorry, I had been focusing on the other change. Could you explain why this part is needed? I'm having trouble seeing how q or p can end up being NULL.

This revision now requires review to proceed.Sat, Mar 27, 6:01 PM
This revision is now accepted and ready to land.Sat, Mar 27, 6:02 PM
cddl/contrib/opensolaris/common/ctf/ctf_lookup.c
94

Sorry, I had been focusing on the other change. Could you explain why this part is needed? I'm having trouble seeing how q or p can end up being NULL.

94

You're right. I've updated the diff to reflect it. I think it was left over from debugging when I thought my code had caused a memory corruption :-).