[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230215053748.GA11780@Jiazi.Li>
Date: Wed, 15 Feb 2023 13:49:53 +0800
From: lijiazi <jqqlijiazi@...il.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>,
David Rientjes <rientjes@...gle.com>,
Joonsoo Kim <iamjoonsoo.kim@....com>,
Andrew Morton <akpm@...ux-foundation.org>,
Roman Gushchin <roman.gushchin@...ux.dev>,
Hyeonggon Yoo <42.hyeyoo@...il.com>,
"Jiazi.Li" <jiazi.li@...nssion.com>, linux-mm@...ck.org,
Kees Cook <keescook@...omium.org>,
Kernel Hardening <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH] mm/slab: always use cache from obj
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@...nssion.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",
>
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
