Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <623a87c6-c0d2-799a-c39e-0d14dcdfa6df@suse.cz>
Date:   Tue, 23 May 2023 09:46:46 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH] mm/slab: remove HAVE_HARDENED_USERCOPY_ALLOCATOR
Content-Language: en-US
To:     Lorenzo Stoakes <lstoakes@gmail.com>
Cc:     Christoph Lameter <cl@linux.com>,
        Pekka Enberg <penberg@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        Hyeonggon Yoo <42.hyeyoo@gmail.com>,
        Kees Cook <keescook@chromium.org>, linux-mm@kvack.org,
        linux-hardening@vger.kernel.org, patches@lists.linux.dev,
        linux-kernel@vger.kernel.org
References: <20230523073136.4900-1-vbabka@suse.cz>
 <310077ed-6f3f-41fe-afcf-36500a9408ec@lucifer.local>
From:   Vlastimil Babka <vbabka@suse.cz>
In-Reply-To: <310077ed-6f3f-41fe-afcf-36500a9408ec@lucifer.local>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/23/23 09:42, Lorenzo Stoakes wrote:
> On Tue, May 23, 2023 at 09:31:36AM +0200, Vlastimil Babka wrote:
>> With SLOB removed, both remaining allocators support hardened usercopy,
>> so remove the config and associated #ifdef.
>>
>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>> ---
>>  mm/Kconfig       | 2 --
>>  mm/slab.h        | 9 ---------
>>  security/Kconfig | 8 --------
>>  3 files changed, 19 deletions(-)
>>
>> diff --git a/mm/Kconfig b/mm/Kconfig
>> index 7672a22647b4..041f0da42f2b 100644
>> --- a/mm/Kconfig
>> +++ b/mm/Kconfig
>> @@ -221,7 +221,6 @@ choice
>>  config SLAB
>>  	bool "SLAB"
>>  	depends on !PREEMPT_RT
>> -	select HAVE_HARDENED_USERCOPY_ALLOCATOR
>>  	help
>>  	  The regular slab allocator that is established and known to work
>>  	  well in all environments. It organizes cache hot objects in
>> @@ -229,7 +228,6 @@ config SLAB
>>
>>  config SLUB
>>  	bool "SLUB (Unqueued Allocator)"
>> -	select HAVE_HARDENED_USERCOPY_ALLOCATOR
>>  	help
>>  	   SLUB is a slab allocator that minimizes cache line usage
>>  	   instead of managing queues of cached objects (SLAB approach).
>> diff --git a/mm/slab.h b/mm/slab.h
>> index f01ac256a8f5..695ef96b4b5b 100644
>> --- a/mm/slab.h
>> +++ b/mm/slab.h
>> @@ -832,17 +832,8 @@ struct kmem_obj_info {
>>  void __kmem_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *slab);
>>  #endif
>>
>> -#ifdef CONFIG_HAVE_HARDENED_USERCOPY_ALLOCATOR
>>  void __check_heap_object(const void *ptr, unsigned long n,
>>  			 const struct slab *slab, bool to_user);
>> -#else
>> -static inline
>> -void __check_heap_object(const void *ptr, unsigned long n,
>> -			 const struct slab *slab, bool to_user)
>> -{
>> -}
>> -#endif
> 
> Hm, this is still defined in slab.c/slub.c and invoked in usercopy.c, do we
> not want the prototype?

Well I didn't delete the prototype, just the ifdef/else around, so now it's
there unconditionally.

> Perhaps replacing with #ifdef
> CONFIG_HARDENED_USERCOPY instead? I may be missing something here :)

Putting it under that #ifdef would work and match that the implementations
of that function are under that same ifdef, but maybe it's unnecessary noise
in the header?