linux-kernel - Re: [PATCH 1/2] mm/slab_common: add SLAB_NO_MERGE flag for use when creating slabs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150902011524.GM26895@dastard>
Date:	Wed, 2 Sep 2015 11:15:24 +1000
From:	Dave Chinner <david@...morbit.com>
To:	Mike Snitzer <snitzer@...hat.com>
Cc:	linux-kernel@...r.kernel.org, linux-mm@...ck.org, riel@...hat.com,
	axboe@...nel.dk, dm-devel@...hat.com, anderson@...hat.com
Subject: Re: [PATCH 1/2] mm/slab_common: add SLAB_NO_MERGE flag for use when
 creating slabs

On Tue, Sep 01, 2015 at 01:51:29PM -0400, Mike Snitzer wrote:
> The slab aliasing/merging by default transition went unnoticed (at least
> to the DM subsystem).  Add a new SLAB_NO_MERGE flag that allows
> individual slabs to be created without slab merging.  This beats forcing
> all slabs to be created in this fashion by specifying sl[au]b_nomerge on
> the kernel commandline.

I didn't realise that this merging had also been applied to SLAB - I
thought it was just SLUB that did this.  Indeed, one of the prime
reasons for using SLAB over SLUB was that it didn't merge caches and
so still gave excellent visibility of runtime slab memory usage on
production systems.

I had no idea that commit 12220de ("mm/slab: support slab merge")
had made SLAB do this as well as it was not cc'd to any of the
people/subsystems that maintain code that uses slab caches.  IMNSHO
the commit message gives pretty flimsy justification for such a
change, especially considering we need to deal with slab caches that
individually grow to contain hundreds of millions of objects.

> DM has historically taken care to have separate named slabs that each
> devices' mempool_t are backed by.  These separate slabs are useful --
> even if only to aid inspection of DM's memory use (via /proc/slabinfo)
> on production systems.

Yup, that's one of the reasons XFS has 17 separate slab caches. The
other main reason is that slab separation also helps keep memory
corruption and use-after free issues contained; if you have a
problem with the objects from one slab cache, the isolation of the
slab makes it less likely that the problem propagates to other
subsystems. Hence failures also tend to be isolated to the code that
has the leak/corruption problem, making them easier to triage and
debug on production systems...

> I stumbled onto slab merging as a side-effect of a leak in dm-cache
> being attributed to 'kmalloc-96' rather than the expected
> 'dm_bio_prison_cell' named slab.  Moving forward DM will disable slab
> merging for all of DM's slabs by using SLAB_NO_MERGE.

Seems like a fine idea to me. I can apply it to various slabs in XFS
once it's merged....

Acked-by: Dave Chinner <dchinner@...hat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/