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: <6b41bb2c-6305-2bf4-1949-84ba08fdbd72@suse.cz>
Date:   Thu, 11 Aug 2022 12:07:40 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.1.0
From:   vbabka@suse.cz
Subject: Re: [PATCH v2] Introduce sysfs interface to disable kfence for
 selected slabs.
Content-Language: en-US
To:     Marco Elver <elver@google.com>
Cc:     Imran Khan <imran.f.khan@oracle.com>, glider@google.com,
        dvyukov@google.com, cl@linux.com, penberg@kernel.org,
        rientjes@google.com, iamjoonsoo.kim@lge.com,
        akpm@linux-foundation.org, roman.gushchin@linux.dev,
        42.hyeyoo@gmail.com, linux-kernel@vger.kernel.org,
        kasan-dev@googlegroups.com, linux-mm@kvack.org
References: <20220811085938.2506536-1-imran.f.khan@oracle.com>
 <d3cd0f34-b30b-9a1d-8715-439ffb818539@suse.cz>
 <CANpmjNMYwxbkOc+LxLfZ--163yfXpQj69oOfEFkSwq7JZurbdA@mail.gmail.com>
In-Reply-To: <CANpmjNMYwxbkOc+LxLfZ--163yfXpQj69oOfEFkSwq7JZurbdA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 8/11/22 11:52, Marco Elver wrote:
> On Thu, 11 Aug 2022 at 11:31, <vbabka@suse.cz> wrote:
>>
>> On 8/11/22 10:59, Imran Khan wrote:
>> > By default kfence allocation can happen for any slab object, whose size
>> > is up to PAGE_SIZE, as long as that allocation is the first allocation
>> > after expiration of kfence sample interval. But in certain debugging
>> > scenarios we may be interested in debugging corruptions involving
>> > some specific slub objects like dentry or ext4_* etc. In such cases
>> > limiting kfence for allocations involving only specific slub objects
>> > will increase the probablity of catching the issue since kfence pool
>> > will not be consumed by other slab objects.
>>
>> So you want to enable specific caches for kfence.
>>
>> > This patch introduces a sysfs interface '/sys/kernel/slab/<name>/skip_kfence'
>> > to disable kfence for specific slabs. Having the interface work in this
>> > way does not impact current/default behavior of kfence and allows us to
>> > use kfence for specific slabs (when needed) as well. The decision to
>> > skip/use kfence is taken depending on whether kmem_cache.flags has
>> > (newly introduced) SLAB_SKIP_KFENCE flag set or not.
>>
>> But this seems everything is still enabled and you can selectively disable.
>> Isn't that rather impractical?
> 
> A script just iterates through all the caches that they don't want,
> and sets skip_kfence? It doesn't look more complicated.

Well, yeah, it's possible.

>> How about making this cache flag rather denote that KFENCE is enabled (not
>> skipped), set it by default only for for caches with size <= 1024, then you
> 
> Where does 1024 come from? PAGE_SIZE?

You're right, the existing check in __kfence_alloc() uses PAGE_SIZE, not
1024, which probably came from lack of coffee :)

> The problem with that opt-in vs. opt-out is that it becomes more
> complex to maintain opt-in (as the first RFC of this did). With the

I see. There was a kfence_global_alloc_enabled and slub_kfence[=slabs] ...
that probably wouldn't be necessary even in an opt-in scenario as I described.

> new flag SLAB_SKIP_KFENCE, it also can serve a dual purpose, where
> someone might want to explicitly opt out by default and pass it to
> kmem_cache_create() (for whatever reason; not that we'd encourage
> that).

Right, not be able to do that would be a downside (although it should be
possible even with opt-in to add an opt-out cache flag that would just make
sure the opt-in flag is not set even if eligible by global defaults).

> I feel that the real use cases for selectively enabling caches for
> KFENCE are very narrow, and a design that introduces lots of
> complexity elsewhere, just to support this feature cannot be justified
> (which is why I suggested the simpler design here back in
> https://lore.kernel.org/lkml/CANpmjNNmD9z7oRqSaP72m90kWL7jYH+cxNAZEGpJP8oLrDV-vw@mail.gmail.com/
> )

I don't mind strongly either way, just a suggestion to consider.