From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0813BC4345F for ; Thu, 2 May 2024 17:50:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 676376B0085; Thu, 2 May 2024 13:50:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6260D6B0088; Thu, 2 May 2024 13:50:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4C6706B0089; Thu, 2 May 2024 13:50:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 2CEAD6B0085 for ; Thu, 2 May 2024 13:50:44 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id C6B43C0F25 for ; Thu, 2 May 2024 17:50:43 +0000 (UTC) X-FDA: 82074195966.06.0A01F1C Received: from mail-pg1-f181.google.com (mail-pg1-f181.google.com [209.85.215.181]) by imf02.hostedemail.com (Postfix) with ESMTP id D292780015 for ; Thu, 2 May 2024 17:50:41 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=CI2RdvSw; spf=pass (imf02.hostedemail.com: domain of keescook@chromium.org designates 209.85.215.181 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1714672242; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=+uaF0GOsku/Enw0IBZ0DNja/DdL69X77dW5yUv4AwZo=; b=IQeiPIlgOZ5W0PBMg+jlLYOKg0gposLXZJVo9d5s7xx8hZOIlIgUrTLrE8fZ0b7SIlMVYv qZybXrmXPIqvlFaChN0BaD40hKnRm/RjuCtbT4a6xYm8xPy35Wxdbycel7MzOy7Fai5DJV 37eIIkGOiq8sTICoCDxqvGhWvdIndGY= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=CI2RdvSw; spf=pass (imf02.hostedemail.com: domain of keescook@chromium.org designates 209.85.215.181 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1714672242; a=rsa-sha256; cv=none; b=l3KSH5mjiUSJtyjLxljYJP81P9/AY6yxEwW89VT4rGG9uJnKjBbBdp7LvJ191VxHb77csD TXnMiXAPN4Bau2riNmvNAv90p1jV2jOTYVkqP+LL3BgsSkVMwvGlDWcNOZNzn/UWVaGb4F ViOq5QLHfIQ5Xaf0IjFKWeea2TrGqcQ= Received: by mail-pg1-f181.google.com with SMTP id 41be03b00d2f7-5ff57410ebbso6118160a12.1 for ; Thu, 02 May 2024 10:50:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1714672241; x=1715277041; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=+uaF0GOsku/Enw0IBZ0DNja/DdL69X77dW5yUv4AwZo=; b=CI2RdvSwG1lVXhk530v836nqHaQbzsIytgkO4AAEelDZm46XXuIrPA89YsXeTtAOCo Mrj1mJ9gN4bvIscMSwNqZOgZMF0Pc3yVRmGBoKyCZIYopGrxxO/3Ku0q31+qvuHOFh5n cpzyhOMKIH3LuIkBQIwe6Yzr8dhMielT6QcA0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714672241; x=1715277041; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=+uaF0GOsku/Enw0IBZ0DNja/DdL69X77dW5yUv4AwZo=; b=ofvgYnjLwK450rfVPXrH/+TIRAkg6soTFNI7vS81yWF1FUVaMSqXUgZQ8HSnMdn+qN g+/MgPFmrlEcsFxhefun9k1dYgmeWUBXhnNspNyWxZS1pOB/YJYEM3bN1d1Pl9jjaJrD 2wQefb2dLOwgiN5bVB1Ei0Rg+GuaiSTySJ+k2uW3WA20EJdAAtMjBpHCoUMpKU7jY1vn JrwbbB4QDsIOWzvBUvUi/skYmF21O6bfUy7R8v9RFTQFyCMuMAc3FJ74Ud0iGnzBl8Kf pZBqxZMoO/04s0BippZ8jhUTu7fTzhTTeGuXO98oc8z/43H7HCiMZEJ80kcuSbkWRGMV fS/A== X-Forwarded-Encrypted: i=1; AJvYcCUzyiyaRlFeOUUwOBR3mivwuNQnIChHc/jfBPVYDExuBTRqGRCBIw1hBKDQSFUzUYRfWuPikpSOnLfTZ6MFenqzp/U= X-Gm-Message-State: AOJu0YzNhVkt/EhLKrmRYqDM+kbCoqBbb3y+UvEa88o49mgVlA/nnB1P vW+PWHos7AXbSF7UM56ZfRWRrhK2cD1WPd+5/5rKfnDqG/euOx7ZfGJYBOWvp3VlZu/CCE9P/pU = X-Google-Smtp-Source: AGHT+IEopB5Z+Ot/+/0juPZG0PnqSIGb9G/R6G3qibniQnQojE/g26Yasy+fM+manPwub9pyTkHN6Q== X-Received: by 2002:a17:90a:890d:b0:2af:8fa4:40e with SMTP id u13-20020a17090a890d00b002af8fa4040emr559066pjn.1.1714672240673; Thu, 02 May 2024 10:50:40 -0700 (PDT) Received: from www.outflux.net ([198.0.35.241]) by smtp.gmail.com with ESMTPSA id g5-20020a17090a828500b0029df9355e79sm1540941pjn.13.2024.05.02.10.50.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 May 2024 10:50:40 -0700 (PDT) Date: Thu, 2 May 2024 10:50:39 -0700 From: Kees Cook To: Allen Pais Cc: 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, mcgrof@kernel.org, j.granados@samsung.com Subject: Re: [PATCH v2] fs/coredump: Enable dynamic configuration of max file note size Message-ID: <202405021045.360F5313EA@keescook> References: <20240502145920.5011-1-apais@linux.microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240502145920.5011-1-apais@linux.microsoft.com> X-Rspamd-Queue-Id: D292780015 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: mc73mhh7hrtgqnnhom8138xqtf5tbddh X-HE-Tag: 1714672241-558335 X-HE-Meta: U2FsdGVkX1/KFGbvj+7QGZWcjN9wUYh3QdtzlWRuKaSr1lJ2ZUfJHD/zv19cGtxhRmX9NQTROZxbu8a47767YLc0S6+MY9TwOmfcTgoHDuRq0FD0zQbQeRVg2tcLKkb2S3/jFVuskM86T1ORaxTQGxwBFWjxz0qKuH6AHg1SePnJGgIwOGNozGJCyY7snUsKYLXZOyV84nG3s5PHjWlDYO6zQGrwnHd8QA3ZtsgGUEoYWsfXYK0PTKsGdU0ApgSkQYUlHdClucL2hku6g4Lk1xjehClY6B/hCDZRRdO20MXUlTQoUtrd+QGvjuQMEKw+6oAvcvHVVXNqRJ9rt/meH7yY9qiloTsS2q2ob2/DgnPlOvY0jG5rVjV4isJ5cT97GSrl9UGpVTAkFCGO+Ga/LikbPZhOlREw7ZYIMHFefXi+5DOb3iDJqzXUGUutKEbOHOcP890TdOQrDS1dhYX0C4I3mDYPZhjOlevKl8h5orJYQW5hhP8rErQbB8lfufqh+Hitde1xFap2QBbmukNIB2fvR9qLyADIBxEINyWX/0W33J74sTUPhbWHfFSC/O5GTDa/Z9kERKR5pm8hgpvRPM5szgl7GXq/rmpozwQ51Z+VwGBlExsOd9jirmN6JW3HMiWq1dnSSefPrEK6CYaFhpz4IHUjodPfS1/GJ1f9u9E5EoNwi34dJIvW/Zb5L1KkDQ9hGoPZ/X970sfr9m8MjIfkP0hElwO1toHuXDEZXGRCKyxPVil8RfvkGisSs2bUvCfJ/QW+CQCwd3XQl6i7FM1+u5YAm+YXbU9YLuqcn8VWdM8DgLwcPuqwMTiU+5s9lJoTo+wsqVUoxISCV6mCalvu9amYBfhKkE9WbVTMmfUeCYKQq0Kv61a89WYpvSz2VmKL1MkytuyV0pvkwbxBhPNZyr+Qnme/pFzp+8ZTxibWAzunjQczeGS+4pqn+1LvwHXnIDIhtFq097jDNxS LCFj8vE1 NfTLetcRFwQhAoXavk5mRWjpul5v71ZxDWOIAs6ulIR8Nz8zrG+byqbaxCsZwNYUinif/NxycAdgKutORHC8F6UXAKmcFQbJcm9OGCHfgMUT47o4uQshXAOuv9RIG+404wliejTJzXfKrEJ4X+HYuQFhp8yh7zwk8sQMIZqsZ3MiftTSYnzKxoWAl+Cv7m8XKFcvnxkes/vwQzA7lsHaKo5XAm7zvBA1PxTgaBnepPpUh+wn5PeuYBGlzHApgJ73BD77ki6LygDMTsazCJbeOMgJZzf7+J1ldpZs1EJdwq2X/NUAbz7sXt6DVXr1FrwSF1ZJ4ASghhoFEJhPWmlKGgMn55GDXVNXHthgRgZ2GLsIbbvyZE9mlPwuLQusjqAjzoALRA1vrd+6a8f6Dsk6xJaC6rb2mkYy2+4UX X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, May 02, 2024 at 02:59:20PM +0000, Allen Pais wrote: > Introduce the capability to dynamically configure the maximum file > note size for ELF core dumps via sysctl. This enhancement removes > the previous static limit of 4MB, allowing system administrators to > adjust the size based on system-specific requirements or constraints. > > - Remove hardcoded `MAX_FILE_NOTE_SIZE` from `fs/binfmt_elf.c`. > - Define `max_file_note_size` in `fs/coredump.c` with an initial value > set to 4MB. > - Declare `max_file_note_size` as an external variable in > `include/linux/coredump.h`. > - Add a new sysctl entry in `kernel/sysctl.c` to manage this setting > at runtime. > > $ sysctl -a | grep max_file_note_size > kernel.max_file_note_size = 4194304 > > $ sysctl -n kernel.max_file_note_size > 4194304 > > $echo 519304 > /proc/sys/kernel/max_file_note_size > > $sysctl -n kernel.max_file_note_size > 519304 The names and paths in the commit log need a refresh here, since they've changed. > > 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, > leading to incomplete memory segment information in the resulting coredump. > This truncation compromises the utility of the coredumps, as crucial > information about the memory state at the time of the crash might be > omitted. Thanks for adding this! > > Signed-off-by: Vijay Nag > Signed-off-by: Allen Pais > > --- > Changes in v2: > - Move new sysctl to fs/coredump.c [Luis & Kees] > - rename max_file_note_size to core_file_note_size_max [kees] > - Capture "why this is being done?" int he commit message [Luis & Kees] > --- > fs/binfmt_elf.c | 3 +-- > fs/coredump.c | 10 ++++++++++ > include/linux/coredump.h | 1 + > 3 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > index 5397b552fbeb..6aebd062b92b 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -1564,7 +1564,6 @@ static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata, > fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata); > } > > -#define MAX_FILE_NOTE_SIZE (4*1024*1024) > /* > * Format of NT_FILE note: > * > @@ -1592,7 +1591,7 @@ static int fill_files_note(struct memelfnote *note, struct coredump_params *cprm > > names_ofs = (2 + 3 * count) * sizeof(data[0]); > alloc: > - if (size >= MAX_FILE_NOTE_SIZE) /* paranoia check */ > + if (size >= core_file_note_size_max) /* paranoia check */ > return -EINVAL; I wonder, given the purpose of this sysctl, if it would be a discoverability improvement to include a pr_warn_once() before the EINVAL? Like: /* paranoia check */ if (size >= core_file_note_size_max) { pr_warn_once("coredump Note size too large: %zu (does kernel.core_file_note_size_max sysctl need adjustment?\n", size); return -EINVAL; } What do folks think? (I can't imagine tracking down this problem originally was much fun, for example.) > size = round_up(size, PAGE_SIZE); > /* > diff --git a/fs/coredump.c b/fs/coredump.c > index be6403b4b14b..a312be48030f 100644 > --- a/fs/coredump.c > +++ b/fs/coredump.c > @@ -56,10 +56,13 @@ > static bool dump_vma_snapshot(struct coredump_params *cprm); > static void free_vma_snapshot(struct coredump_params *cprm); > > +#define MAX_FILE_NOTE_SIZE (4*1024*1024) > + > static int core_uses_pid; > static unsigned int core_pipe_limit; > static char core_pattern[CORENAME_MAX_SIZE] = "core"; > static int core_name_size = CORENAME_MAX_SIZE; > +unsigned int core_file_note_size_max = MAX_FILE_NOTE_SIZE; > > struct core_name { > char *corename; > @@ -1020,6 +1023,13 @@ static struct ctl_table coredump_sysctls[] = { > .mode = 0644, > .proc_handler = proc_dointvec, > }, > + { > + .procname = "core_file_note_size_max", > + .data = &core_file_note_size_max, > + .maxlen = sizeof(unsigned int), > + .mode = 0644, > + .proc_handler = proc_douintvec, > + }, > }; > > static int __init init_fs_coredump_sysctls(void) > diff --git a/include/linux/coredump.h b/include/linux/coredump.h > index d3eba4360150..14c057643e7f 100644 > --- a/include/linux/coredump.h > +++ b/include/linux/coredump.h > @@ -46,6 +46,7 @@ static inline void do_coredump(const kernel_siginfo_t *siginfo) {} > #endif > > #if defined(CONFIG_COREDUMP) && defined(CONFIG_SYSCTL) > +extern unsigned int core_file_note_size_max; > extern void validate_coredump_safety(void); > #else > static inline void validate_coredump_safety(void) {} > -- > 2.17.1 Otherwise, yes, this looks good to me. -- Kees Cook