Re: [PATCH] mm/slab: always use cache from obj

[lijiazi <jqqlijiazi@gmail.com>]

On Tue, Feb 14, 2023 at 11:33:21AM +0100, Vlastimil Babka wrote:
> On 2/14/23 11:19, Jiazi.Li wrote:
> > If free obj to a wrong cache, in addition random, different offset
> > and object_size will also cause problems:
> > 1. The offset of a cache with a ctor is not zero, free an object from
> > this cache to cache with offset zero, will write next freepointer to
> > wrong location, resulting in confusion of freelist.
> 
> Kernels hardened against freelist corruption will enable
> CONFIG_SLAB_FREELIST_HARDENED, so that's already covered, no?
> 
Yes, HARDENED already covered.
> > 2. If wrong cache want init on free, and cache->object_size is large
> > than obj size, which may lead to overwrite issue.
> 
> In general, being defensive against usage errors is part of either hardening
> or debugging, which is what the existing code takes into account.
> 
My consideration is for the wrong cache problem on version without
HARDENED or debugging, it is likely to cause kernel panic, and such
problem is difficult to analyze on non-debug version.
When reproducing this problem on debug version, it will not cause kernel
panic, but only print the WARN log, then use correct cache to free obj.
Because we want to reproduce kernel panic problem, so may ignore WARN
log and think that can not reproduce problem on debug version.

Thanks for your reply, I will enable CONFIG_SLAB_FREELIST_HARDENED on
non-debug version later.
> > Compared with adding a lot of if-else, it may be better to use obj's
> > cache directly.
> > 
> > Signed-off-by: Jiazi.Li <jiazi.li@transsion.com>
> > ---
> >  mm/slab.h | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/mm/slab.h b/mm/slab.h
> > index 63fb4c00d529..ed39b2e4f27b 100644
> > --- a/mm/slab.h
> > +++ b/mm/slab.h
> > @@ -670,10 +670,6 @@ static inline struct kmem_cache *cache_from_obj(struct kmem_cache *s, void *x)
> >  {
> >  	struct kmem_cache *cachep;
> >  
> > -	if (!IS_ENABLED(CONFIG_SLAB_FREELIST_HARDENED) &&
> > -	    !kmem_cache_debug_flags(s, SLAB_CONSISTENCY_CHECKS))
> > -		return s;
> > -
> >  	cachep = virt_to_cache(x);
> >  	if (WARN(cachep && cachep != s,
> >  		  "%s: Wrong slab cache. %s but object is from %s\n",
>