Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <bb6ad3ee-8d00-bdd5-3f50-a8f836892d51@suse.cz>
Date: Tue, 14 Feb 2023 11:33:21 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: "Jiazi.Li" <jqqlijiazi@...il.com>, 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>
Cc: "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 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?

> 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.

> 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.