Get rid of superfluous "bug marker" code in the bcache
ClosedPublic
Actions

Authored by sobomax on Nov 22 2018, 6:16 PM.

Details

Reviewers

tsoome
allanjude

Commits

rS340857: Nuke out buffer overflow safety marker code, it duplicates similar code in

Summary

Read-ahead code introduced in the r313332 contains "the end of cache marker" which I believe superfluous, as cache is allocated on heap and there is already a similar code in the malloc()/free() to detect data corruption and panic:

$ grep USEENDGUARD /usr/src/stand/libsa/*.[ch]
/usr/src/stand/libsa/zalloc_defs.h:#define USEENDGUARD
/usr/src/stand/libsa/zalloc_malloc.c:#ifdef USEENDGUARD
/usr/src/stand/libsa/zalloc_malloc.c:#ifdef USEENDGUARD
/usr/src/stand/libsa/zalloc_malloc.c:#ifdef USEENDGUARD

I believe that code is more "correct" since such incidents should not be taken lightly as often loader logs are not easily available and or often looked upon. Merely issuing a warning when condition is detected is not sufficient since allowing system to continue bootstrap is likely to cause a subsequent kernel crash/misbehaviour that would be really hard to debug and can lead to a severe data corruption as well.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 21161

Event Timeline

sobomax created this revision.Nov 22 2018, 6:16 PM

Herald added a subscriber: imp. · View Herald TranscriptNov 22 2018, 6:16 PM

Harbormaster completed remote builds in B21161: Diff 50920.Nov 22 2018, 6:16 PM

sobomax edited the summary of this revision. (Show Details)Nov 22 2018, 6:18 PM

Good by me - I did add this for extra safety because the malloc markers are checked on free. And since we havent been bitten yet, I guess we are good:)

This revision is now accepted and ready to land.Nov 22 2018, 6:22 PM

sobomax edited the summary of this revision. (Show Details)Nov 22 2018, 6:22 PM

Closed by commit rS340857: Nuke out buffer overflow safety marker code, it duplicates similar code in (authored by sobomax). · Explain WhyNov 23 2018, 10:37 PM

This revision was automatically updated to reflect the committed changes.