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 9FDC9C4345F for ; Mon, 29 Apr 2024 22:32:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 06A636B0088; Mon, 29 Apr 2024 18:32:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 01AE76B0089; Mon, 29 Apr 2024 18:32:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E23BD6B008A; Mon, 29 Apr 2024 18:32:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id C3F856B0088 for ; Mon, 29 Apr 2024 18:32:45 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 46AA7402F9 for ; Mon, 29 Apr 2024 22:32:45 +0000 (UTC) X-FDA: 82064020290.02.487D6B7 Received: from mail-ua1-f50.google.com (mail-ua1-f50.google.com [209.85.222.50]) by imf14.hostedemail.com (Postfix) with ESMTP id 84BD0100012 for ; Mon, 29 Apr 2024 22:32:43 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=BlWrJtqq; spf=pass (imf14.hostedemail.com: domain of allen.lkml@gmail.com designates 209.85.222.50 as permitted sender) smtp.mailfrom=allen.lkml@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1714429963; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=kkJIofc7sCacjx+o3Bhnh3utDqU2SSgA8jzJYUliTOM=; b=WhMgBC6rId4jSTCcXje2yiZj1EXh41qMsRanZzekL5dF6XefLEqexCUjrP64SRbXcemKTS RoFEPRddqHPe6R0Vn6UZTiWFLUTa0McfJY+N+xItMHw8Sj0QfwcHF7WX1142ZLoW81Isrw iogxizCJQUDaeceGEYesHVXIvCL9qiw= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=BlWrJtqq; spf=pass (imf14.hostedemail.com: domain of allen.lkml@gmail.com designates 209.85.222.50 as permitted sender) smtp.mailfrom=allen.lkml@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1714429963; a=rsa-sha256; cv=none; b=EOinbHwusdgr+z2NPhM5l4l8OIuXwjcLlrVbxEsrD4hgAPlKsXaBuTcSl8QW+L0IyL85cE oZ1L3Jof2KJImmdduHfvwqrcb2jJOKlQBidwOKNL314pweIgkLEgZcfBhYk8jaOCMnwhpk rd2zvXiIRuhtLVOicYCbUC/Mhv2IeXo= Received: by mail-ua1-f50.google.com with SMTP id a1e0cc1a2514c-7f169d3ef53so113628241.1 for ; Mon, 29 Apr 2024 15:32:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1714429962; x=1715034762; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=kkJIofc7sCacjx+o3Bhnh3utDqU2SSgA8jzJYUliTOM=; b=BlWrJtqqFQgo8yUSq4X6WDTjC81DUHhNkUjPb6AlneOPTLG/L4dS/fHBUs7pr5OIhH /301l1pggD5FUlGaK3cpyqP+5r4Cqja5P5tjRN51Q7+hcrCPSXRqu1MVzXrqDO/TudrG RN5KTDJTTPLt2qXDLXVmcqMSYCZHQY3HMUe/ozOm5Yyj46r3o2X2QWqTh30IINxaAoyP nbqcc7pdB8P5LYMKA5SfFkW6cKkm5mp7pPA7RmVNusyKBevE5yzFiGn3N7U+xSQZuCtR RK6OJR/uXxA64pOjl2iUYe/Hf+g0tKxSNRNHee4eoFtTOhVTcMEgA/kpN6AkdtH1+TdO ElIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714429962; x=1715034762; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=kkJIofc7sCacjx+o3Bhnh3utDqU2SSgA8jzJYUliTOM=; b=utVkkiaNYBg9JC9zkZgZOsgrAiFMpBvl9ABiZkkFYfJ1sRE7joUCmEiiFgJbOAz5vZ K/rgnRYeXHClcEa61zkxjpDedHc7mumOGtMQ5WtVGdRYXi5C+1Jp/1+jOJ5hZSG7dr7n rnWP36iTUVxN3wYNcrsYw0YidNma5x197Q4zsrCBl83l09gc3A8TKWJcaB6Uc6O8TlhI Xgqnq5LwPHndNXtnnHktCr0uiJzjGdbUK5JriRGIcpTCuX0f7ZNuUmpiaK9cAJ0x6OwU RMP0otU6MmW+0q1bTSAyod4+GzXpxxxfbADXSjjoQToi3wF+l6L9diZLqU4GNCiuUuxR 2cyA== X-Forwarded-Encrypted: i=1; AJvYcCWrlBgYRoTG+gilFa5mZo/spgfLOHGhUQY8Hrv3XC+BfzAwxxa2vjN46y/8kxVD2UyU3JpxV1lGQN5Q5AeerpkQUYI= X-Gm-Message-State: AOJu0YwyZ7EiUFGV/p10U8FLlxNCymlq9+gdCeCnb/yxE2amhPRWdbZF 41JIhyyhW7xgf36p/8NguCuuKLqGY5NrhQBrqJ4nrLXGwuH/jKU54vkt/y/AGwNS6X5LFzRSGCr SJBTrEzLcdW/lvH3BhFPIoVFUte4= X-Google-Smtp-Source: AGHT+IGKchtg51G2XH8ZwTYZxKRYw/SHqQw3ypfHaPs1T6l41y11+ZRLuYxdEw4Z+cOtExre4q50Kl3T+0HHClwvz3A= X-Received: by 2002:a05:6122:2522:b0:4d8:7359:4c25 with SMTP id cl34-20020a056122252200b004d873594c25mr12797577vkb.12.1714429962449; Mon, 29 Apr 2024 15:32:42 -0700 (PDT) MIME-Version: 1.0 References: <20240429172128.4246-1-apais@linux.microsoft.com> In-Reply-To: From: Allen Date: Mon, 29 Apr 2024 15:32:31 -0700 Message-ID: Subject: Re: [RFC PATCH] fs/coredump: Enable dynamic configuration of max file note size To: Luis Chamberlain Cc: Allen Pais , 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, keescook@chromium.org, j.granados@samsung.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 84BD0100012 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: h9hama4kzxmxnkspaj4w4ghmb7b3o37g X-HE-Tag: 1714429963-468051 X-HE-Meta: U2FsdGVkX19zRa6RVM8MFWoZRtN8WkpL3irw0X2VwYzfkol1pjfVvzLWKPMWTJvPAbPKmB/O9sq8c47PGDgrVtyXIdo51OBT0imoWWRXKRWpp3JMSBImiP9QQe/WhIHChCkSiXh+hpz+Zc8ehICmo3VsSFCQi76zVBu5TCpUTOFTzIEMYviBrMPNGfDZ55RCeWX4aSe24NpQZ8znqQlbSKZP4Jr1tZC3dvV6L+Vn9n/q9P2P45H4B7aYAYSGCc0aDw+6axUukH94bhCtL2RstVLvFkd9P9E/NhZMmzmyCIgMFiesBOnNAm35ic3XX8c9ioize6u+ai6zVBRLw6OvyWg8Qazdx9b7fneElqYy7GOlAS6CRsye9Vu0NyTevwRwfg5rqye3tD4RSp8B43gUNoV9/1Sih7vLwYfMAK0Aw8jxRJ/VSU+ml5dscClyE1d/ieukRzvuFSPcrgckHm3PSiz69kc3EyZjQ7mvot9i1FAcm5MlTeqxHTSuS+rlQwQ0ySlxfFt+jCA5xdokSGYGmYoZ7C1ktMI1/dexo8s6k1oLV58EWHu9F+hp28MuY98p/788s6uIWKCXIdZmyOsbRvlpUJDwzVDtZDO7/xbJFH6hI0DHhewhm4pFpeFFRWXdt8ne3PGUMrSP1ftbudfR+ZZBJaIyDYpxjNi40ghb+0yM4kS60oArZPZtQ1FrN/KNS9qeR5+IAgxdTe7wMqBgeq0YGiPKnboYnlqWWpKZDYCa9k5yhRFJ8cutuD44gFow8KsC7fjrFZ2i1JAJaccHwK1q9ENaIKIzVQQ3rl6KShXWRGVh1SN4ohpKi0T01HK969EubmVZHXOJw1xcYLyvc2LEHiY/F2lW6OAAQc9SQaVxHfApDfoc5RZfYxwosCO7WdGfrP6Y3s/sb8SY4U0s+QlqrH9h6KaQGYtb4iCgjxAgub8Mc1i0L9lA0M82f/ggOYigum3ay/UwoDlKIPo 7mWEM0Xb Boz2wNQ35INchzAlTA/Ez7eEjR8v/3PNLgmgtrdDN100qkFfrg6s1YinX7JWPvCUKF+Qm6jvoSM2oay81/B/yn/0chypj2rneO1XgZ2MDaxuPYiJodiZfVSr/HpZ9ygzt32CPmuzvncePwvn+sr+1LAzbBBCao9j90AgqCXyv4n1fqyxYWGgHLXy7Xx3Mm/SH/lk1lar0+uSX+n4/9QygoQxysXBL4Gxy2nIijTJunDHGsBy9SamvFkZpZy9taWuNud3RT/f8VV9/fq2Xq3qOUZ5fCuQJ0fRk4zxs7aq9cQuJMTMc6KE4GjHFD9FCnTJ1K6TtcQIbIBJVcsZxL2V+ofv5qepnvC5QX1Ld8WijPMOW1/S8b/plIninHinXd+Mb+V7eGAxXlaGpN55r5aU9c4djenWq8JpFKwtbY0dGPp4IVCWO1ygsoFSusmrUQz2eldeI3Y/okJLkjqI2ycgnyBWSSw== 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 Mon, Apr 29, 2024 at 11:38=E2=80=AFAM Luis Chamberlain wrote: > > On Mon, Apr 29, 2024 at 05:21:28PM +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/linu= x/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 =3D 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 > > This doesn't highlight anything about *why*. So in practice you must've > hit a use case where ELF notes are huge, can you give an example of > that? The commit should also describe that this is only used in the path > of a coredump on ELF binaries via elf_core_dump(). > Yes, I should have captured it. We have observed that during a crash when there are more than 65k mmaps in memory, the existing fixed limit on t= he size of the ELF notes section becomes a bottleneck. The notes section quick= ly reaches its capacity, leading to incomplete memory segment information in t= he resulting coredump. This truncation compromises the utility of the coredump= s, as crucial information about the memory state at the time of the crash might be omitted. I will add the above to the commit message. Hope that addresses your concer= n. > More below. > > > Signed-off-by: Vijay Nag > > Signed-off-by: Allen Pais > > --- > > fs/binfmt_elf.c | 3 +-- > > fs/coredump.c | 3 +++ > > include/linux/coredump.h | 1 + > > kernel/sysctl.c | 8 ++++++++ > > 4 files changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > > index 5397b552fbeb..5fc7baa9ebf2 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 *not= e, struct coredump_params *cprm > > > > names_ofs =3D (2 + 3 * count) * sizeof(data[0]); > > alloc: > > - if (size >=3D MAX_FILE_NOTE_SIZE) /* paranoia check */ > > + if (size >=3D max_file_note_size) /* paranoia check */ > > return -EINVAL; > > size =3D round_up(size, PAGE_SIZE); > > /* > > diff --git a/fs/coredump.c b/fs/coredump.c > > index be6403b4b14b..a83c6cc893fc 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] =3D "core"; > > static int core_name_size =3D CORENAME_MAX_SIZE; > > +unsigned int max_file_note_size =3D MAX_FILE_NOTE_SIZE; > > > > struct core_name { > > char *corename; > > diff --git a/include/linux/coredump.h b/include/linux/coredump.h > > index d3eba4360150..e1ae7ab33d76 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 max_file_note_size; > > extern void validate_coredump_safety(void); > > #else > > static inline void validate_coredump_safety(void) {} > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > > index 81cc974913bb..80cdc37f2fa2 100644 > > --- a/kernel/sysctl.c > > +++ b/kernel/sysctl.c > > @@ -63,6 +63,7 @@ > > #include > > #include > > #include > > +#include > > > > #include "../lib/kstrtox.h" > > > > @@ -1623,6 +1624,13 @@ static struct ctl_table kern_table[] =3D { > > .mode =3D 0644, > > .proc_handler =3D proc_dointvec, > > }, > > + { > > + .procname =3D "max_file_note_size", > > + .data =3D &max_file_note_size, > > + .maxlen =3D sizeof(unsigned int), > > + .mode =3D 0644, > > + .proc_handler =3D proc_dointvec, > > + }, > > #ifdef CONFIG_PROC_SYSCTL > > No, please move this to coredump_sysctls in fs/coredump.c. And there is > no point in supporting int, this is unisgned int right? So use the right > proc handler for it. > Will address it in v2. > If we're gonna do this, it makes sense to document the ELF note binary > limiations. Then, consider a defense too, what if a specially crafted > binary with a huge elf note are core dumped many times, what then? > Lifting to 4 MiB puts in a situation where abuse can lead to many silly > insane kvmalloc()s. Is that what we want? Why? > You raise a good point. I need to see how we can safely handle this case. Thanks, Allen > Luis --=20 - Allen