Received-SPF: pass (google.com: domain of linux-kernel+bounces-168423-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Fri, 3 May 2024 16:43:37 -0700
From: Kees Cook <keescook@chromium.org>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: Allen Pais <apais@linux.microsoft.com>, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	viro@zeniv.linux.org.uk, brauner@kernel.org, jack@suse.cz,
	ebiederm@xmission.com, j.granados@samsung.com, allen.lkml@gmail.com
Subject: Re: [PATCH v3] fs/coredump: Enable dynamic configuration of max file
 note size
Message-ID: <202405031634.77B223D8@keescook>
References: <20240502235603.19290-1-apais@linux.microsoft.com>
 <ZjVAJOsC-EtlIXd6@bombadil.infradead.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZjVAJOsC-EtlIXd6@bombadil.infradead.org>

On Fri, May 03, 2024 at 12:51:00PM -0700, Luis Chamberlain wrote:
> Thanks for the cleanups, this is certainly now in the right direction.
> Generic long term growth questions below.
> 
> On Thu, May 02, 2024 at 11:56:03PM +0000, Allen Pais wrote:
> > Why is this being done?
> > We have observed that during a crash when there are more than 65k mmaps
> > in memory, the existing fixed limit on the size of the ELF notes section
> > becomes a bottleneck. The notes section quickly reaches its capacity,
> 
> I'm not well versed here on how core dumps associate mmaps to ELF notes
> section, can you elaborate? Does each new mmap potentially peg
> information on ELF notes section? Where do we standardize on this? Does
> it also change depending on any criteria of the mmap?

This is all in fs/binfmt_elf.c, fill_note_info(). There's a dump for
each thread's info, and then fill_files_note() (which is what this code
is adjusting) which writes out every filename for any file-map VMAs. The
format of NT_FILE record is documented above fill_files_note(). So, it
all depends on the count of VMAs and length of filenames.

> Depending on the above, we might want to be proactive to get a sense of
> when we want to go beyond the new 16 MiB max cap on new mmaps for instance.
> How many mmaps can we have anyway too?

INT_MAX :)

I'm fine with the new 16MiB max for the coredump. If we really need to
go beyond this, we might need to avoid building the entire thing in
memory, and instead move it all into write_note_info() directly, but I'm
not interested in that refactor unless we have an overwhelmingly good
reason to do so.

-- 
Kees Cook