linux-kernel - Re: [PATCH 4/5] alloc_tag: Track fixed vs dynamic sized kmalloc calls

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <202409111518.9D90EE197@keescook>
Date: Wed, 11 Sep 2024 15:23:00 -0700
From: Kees Cook <kees@...nel.org>
To: Suren Baghdasaryan <surenb@...gle.com>
Cc: Vlastimil Babka <vbabka@...e.cz>,
	Kent Overstreet <kent.overstreet@...ux.dev>,
	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>, linux-mm@...ck.org,
	"GONG, Ruiqi" <gongruiqi@...weicloud.com>,
	Jann Horn <jannh@...gle.com>, Matteo Rizzo <matteorizzo@...gle.com>,
	jvoisin <julien.voisin@...tri.org>,
	Xiu Jianfeng <xiujianfeng@...wei.com>, linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH 4/5] alloc_tag: Track fixed vs dynamic sized kmalloc calls

On Thu, Aug 29, 2024 at 09:00:37AM -0700, Suren Baghdasaryan wrote:
> On Fri, Aug 9, 2024 at 12:33 AM Kees Cook <kees@...nel.org> wrote:
> [...]
> > -#define kmem_cache_alloc(...)                  alloc_hooks(kmem_cache_alloc_noprof(__VA_ARGS__))
> > +#define kmem_cache_alloc(...)          alloc_hooks(kmem_cache_alloc_noprof(__VA_ARGS__))
> 
> nit: seems like an unnecessary churn.

Whoops, yes. This was left over from an earlier pass and I failed to get
the whitespace correctly restored. I will fix this this.

> > diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> > index 81e5f9a70f22..6d2cb72bf269 100644
> > --- a/lib/alloc_tag.c
> > +++ b/lib/alloc_tag.c
> > @@ -78,6 +78,14 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct)
> >
> >         seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls);
> >         codetag_to_text(out, ct);
> > +#ifdef CONFIG_SLAB_PER_SITE
> > +       seq_buf_putc(out, ' ');
> > +       seq_buf_printf(out, "size:%s(%zu) slab:%s",
> > +                               tag->meta.sized == 0 ? "non-slab" :
> 
> "non-slab" term sounds overly specific and we might extend this to
> some other allocations as well in the future. I would suggest
> "unknown" instead.

Heh, yeah. I went back and forth on the name for this and went with
non-slab because we do know what it isn't. It's not some kind of
unexpected state. Maybe "untracked", or "unsized", though both seem
inaccurate from certain perspectives.

> 
> > +                                       tag->meta.sized == SIZE_MAX ? "dynamic" : "fixed",
> > +                               tag->meta.sized == SIZE_MAX ? 0 : tag->meta.sized,
> > +                               tag->meta.cache ? "ready" : "unused");
> 
> I don't see "struct alloc_meta" having a "cache" member...

Oops, yes, as you found this should have been associated with the next
patch that adds "cache".

> Since you are changing the format of this file, you want to also bump
> up the file version inside print_allocinfo_header().

Okay, yeah. In that case I'll probably split the report into a separate
patch after "cache" is added so there's only a single bump in allocinfo
versioning.

-- 
Kees Cook