Page MenuHomeFreeBSD

Check the index hasn't changed after writing the cmp entry
ClosedPublic

Authored by andrew on Feb 21 2019, 8:55 PM.

Details

Summary

If an interrupt fires while writing the cmp entry we may have a partial
entry. Work around this by using atomic_cmpset to set the new index.
If it fails we need to set the previous index value and try again as
the entry may be in an inconsistent state.

This fixes messages similar to the following from syzkaller:
bad comp 224 type 2163727253

Diff Detail

Repository
rS 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

andrew created this revision.Feb 21 2019, 8:55 PM
tuexen accepted this revision.Feb 21 2019, 10:02 PM
This revision is now accepted and ready to land.Feb 21 2019, 10:02 PM
This revision was automatically updated to reflect the committed changes.
cem added a subscriber: cem.Mar 28 2019, 1:18 AM
cem added inline comments.
head/sys/kern/kern_kcov.c
258

Is this backwards? Should it be index = buf[0];?

andrew added inline comments.Mar 28 2019, 8:49 AM
head/sys/kern/kern_kcov.c
258

No, if we get to this point buf[0] has changed since it was loaded in line 244. This can only happen due to an interrupt so should be discarded. As we don't know when the interrupt happened it may return inconsistent data. As such the best way to handle this is to try writing to the same index slot again, however for this to work we need a known value in buf[0] for the cmpset call in line 256.

TL;DR: Once we have the index we need to write to its slot to ensure we get consistent data.