Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 30 Sep 2021 14:42:02 +1000
From:   Dave Chinner <david@fromorbit.com>
To:     Rustam Kovhaev <rkovhaev@gmail.com>
Cc:     djwong@kernel.org, linux-xfs@vger.kernel.org, cl@linux.com,
        penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com,
        akpm@linux-foundation.org, vbabka@suse.cz,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        gregkh@linuxfoundation.org
Subject: Re: [PATCH] xfs: use kmem_cache_free() for kmem_cache objects
Message-ID: <20210930044202.GP2361455@dread.disaster.area>
References: <20210929212347.1139666-1-rkovhaev@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210929212347.1139666-1-rkovhaev@gmail.com>
Precedence: bulk

On Wed, Sep 29, 2021 at 02:23:47PM -0700, Rustam Kovhaev wrote:
> For kmalloc() allocations SLOB prepends the blocks with a 4-byte header,
> and it puts the size of the allocated blocks in that header.
> Blocks allocated with kmem_cache_alloc() allocations do not have that
> header.
> 
> SLOB explodes when you allocate memory with kmem_cache_alloc() and then
> try to free it with kfree() instead of kmem_cache_free().
> SLOB will assume that there is a header when there is none, read some
> garbage to size variable and corrupt the adjacent objects, which
> eventually leads to hang or panic.
> 
> Let's make XFS work with SLOB by using proper free function.
> 
> Fixes: 9749fee83f38 ("xfs: enable the xfs_defer mechanism to process extents to free")
> Signed-off-by: Rustam Kovhaev <rkovhaev@gmail.com>

IOWs, XFS has been broken on SLOB for over 5 years and nobody
anywhere has noticed.

And we've just had a discussion where the very best solution was to
use kfree() on kmem_cache_alloc() objects so we didn't ahve to spend
CPU doing global type table lookups or use an extra 8 bytes of
memory per object to track the slab cache just so we could call
kmem_cache_free() with the correct slab cache.

But, of course, SLOB doesn't allow this and I was really tempted to
solve that by adding a Kconfig "depends on SLAB|SLUB" option so that
we don't have to care about SLOB not working.

However, as it turns out that XFS on SLOB has already been broken
for so long, maybe we should just not care about SLOB code and
seriously consider just adding a specific dependency on SLAB|SLUB...

Thoughts?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com