Maxime Villard <max%m00nbsd.net@localhost> writes: > Le 21/08/2018 à 14:49, Greg Troxel a écrit : >> Maxime Villard <max%m00nbsd.net@localhost> writes: >> >>> Here is a patch [1] that allows kasan to monitor pools and pool_caches. We >>> recycle the existing POOL_REDZONE implementation - which I wrote three years >>> ago, and which has never been enabled (not even on DEBUG). With this we can >>> detect read/write buffer overflows on all our pools, and in particular, on >>> mbufs. >>> >>> I guess people are fine? Otherwise if we prefer to keep a KASAN-independent >>> version of POOL_REDZONE, then we need to enable it under DIAGNOSTIC at least >>> for it to be meaningful. >> >> I don't have a particular opinion about keeping vs not, but DIAGNOSTIC >> should not include anything that causes significant slowdowns, basically >> more than assertions. I don't want to try to get really quantitative, >> but DIAGNOSTIC should run at 95%+ the speed of a regular build, so that >> there's no real performance reason to avoid it. >> >> POOL_REDZONE, if kept, sound fine for DEBUG, where slowdowns are ok. > > I don't think POOL_REDZONE qualifies as causing "significant slowdowns". > That's only a two-byte redzone, which we fill on allocation, and check > on deallocation. So it's like a KASSERT on a uint16_t. > > I haven't measured, but it probably doesn't cost us more than a > KASSERT(mutex_owned()) in the same path. Glad to hear it. That doesn't sound bad. (I had the impression this had extra pages without access permission as guards, or something more serious.) > After more thought I think we should keep it and enable it under > diagnostic. Certainly ok to keep it. As long as this doesn't make people say "I can't run DIAGNOSTIC in production because it's too slow or too piggy with ram", that seems more or less ok.
Attachment:
signature.asc
Description: PGP signature