On Tue, 12 Jan 2021 at 22:16, Andrey Konovalov <andreyknvl@xxxxxxxxxx> wrote: > > On Tue, Jan 12, 2021 at 3:32 PM Marco Elver <elver@xxxxxxxxxx> wrote: > > > > > +/* > > > + * Unlike kasan_check_read/write(), kasan_check_byte() is performed even for > > > + * the hardware tag-based mode that doesn't rely on compiler instrumentation. > > > + */ > > > > We have too many check-functions, and the name needs to be more precise. > > Intuitively, I would have thought this should have access-type, i.e. > > read or write, effectively mirroring a normal access. > > > > Would kasan_check_byte_read() be better (and just not have a 'write' > > variant because we do not need it)? This would restore ksize() closest > > to what it was before (assuming reporting behaviour is fixed, too). > > > > void kasan_poison(const void *address, size_t size, u8 value); > > > void kasan_unpoison(const void *address, size_t size); > > > -bool kasan_check_invalid_free(void *addr); > > > +bool kasan_check(const void *addr); > > > > Definitely prefer shorted names, but we're in the unfortunate situation > > of having numerous kasan_check-functions, so we probably need to be more > > precise. > > > > kasan_check() makes me think this also does reporting, but it does not > > (it seems to only check the metadata for validity). > > > > The internal function could therefore be kasan_check_allocated() (it's > > now the inverse of kasan_check_invalid_free()). > > Re: kasan_check_byte(): > > I think the _read suffix is only making the name longer. ksize() isn't > checking that the memory is readable (or writable), it's checking that > it's addressable. At least that's the intention of the annotation, so > it makes sense to name it correspondingly despite the implementation. > > Having all kasan_check_*() functions both checking and reporting makes > sense, so let's keep the kasan_check_ prefix. > > What isn't obvious from the name is that this function is present for > every kasan mode. Maybe kasan_check_byte_always()? Although it also > seems too long. > > But I'm OK with keeping kasan_check_byte(). This is fine. > Re kasan_check(): > > Here we can use Andrew's suggestion about the name being related to > what the function returns. And also drop the kasan_check_ prefix as > this function only does the checking. > > Let's use kasan_byte_accessible() instead of kasan_check(). Sounds reasonable to me. Thanks, -- Marco
