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 36794C3DA5D for ; Mon, 22 Jul 2024 19:34:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C5A3E6B0089; Mon, 22 Jul 2024 15:34:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C0A2F6B008A; Mon, 22 Jul 2024 15:34:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AF8A46B008C; Mon, 22 Jul 2024 15:34:47 -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 92F856B0089 for ; Mon, 22 Jul 2024 15:34:47 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 1921E1C41C7 for ; Mon, 22 Jul 2024 19:34:47 +0000 (UTC) X-FDA: 82368391014.13.B0BF6B7 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by imf08.hostedemail.com (Postfix) with ESMTP id 436BF16001E for ; Mon, 22 Jul 2024 19:34:45 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=linux.microsoft.com header.s=default header.b=N62yEBdW; dmarc=pass (policy=none) header.from=linux.microsoft.com; spf=pass (imf08.hostedemail.com: domain of romank@linux.microsoft.com designates 13.77.154.182 as permitted sender) smtp.mailfrom=romank@linux.microsoft.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721676851; a=rsa-sha256; cv=none; b=DueCtKsRZ7EL+GiEQlusMCq+nW+X5Yh7LkYUQuYKj0C4CUt+ksMUJmh+4RaqtdiRhwYs6g 6RA9OhbI2mbWn8DdM17cvD2wNZiTRou/Y8VKnb4C5s9MhtSd9HyySV1Cj14OBmdCpZN0O7 qMedcddKvYJJkZ+hO8/wOhGD2zhZUlM= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=linux.microsoft.com header.s=default header.b=N62yEBdW; dmarc=pass (policy=none) header.from=linux.microsoft.com; spf=pass (imf08.hostedemail.com: domain of romank@linux.microsoft.com designates 13.77.154.182 as permitted sender) smtp.mailfrom=romank@linux.microsoft.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721676851; 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=FQmh/EYQknSmmbveLSRuVujcbnn6z8lPRt8o/jsxY58=; b=JZqJ5GvgLQ1aXrvLtSHjhfbIVc0J5jxHl94UoJe6hgZDJ6bw3IvW8pPvHHspe+CCtOsjDK u3p4dLqjcrM9PeBI5L4EyOUv7q23DEHtmHiBXAIrWu0h0bnDnLsrNE+3Bz+eQaHNm4+jds cxokDdTePM5/lGc3f+VT0KE4Dw7EFPk= Received: from [10.137.186.190] (unknown [131.107.159.62]) by linux.microsoft.com (Postfix) with ESMTPSA id C341620B7165; Mon, 22 Jul 2024 12:34:43 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com C341620B7165 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1721676884; bh=FQmh/EYQknSmmbveLSRuVujcbnn6z8lPRt8o/jsxY58=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=N62yEBdW7/cLTjulRYqDD6iK3BoFHzexx4tC0R8sXdK08GwIa76rFsXh+kGP6AlYt 9eqDXlJjXi2JQ9VNHTS/Sam7QwRwFhuKO/amhyelpJaqaIObRg7Ui93xtpaAQHmfGi yKlfLfloCM/qwgptOZ1tKqpUFsUicZ1fmavl7RZw= Message-ID: Date: Mon, 22 Jul 2024 12:34:44 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 1/2] coredump: Standartize and fix logging To: Allen Pais Cc: akpm@linux-foundation.org, ardb@kernel.org, bigeasy@linutronix.de, brauner@kernel.org, ebiederm@xmission.com, jack@suse.cz, Kees Cook , linux-fsdevel@vger.kernel.org, Linux Kernel Mailing List , linux-mm@kvack.org, nagvijay@microsoft.com, oleg@redhat.com, tandersen@netflix.com, vincent.whitchurch@axis.com, viro@zeniv.linux.org.uk, Allen Pais , benhill@microsoft.com, ssengar@microsoft.com, sunilmut@microsoft.com, vdso@hexbites.dev References: <20240718182743.1959160-1-romank@linux.microsoft.com> <20240718182743.1959160-2-romank@linux.microsoft.com> <1AEC6E18-313E-495F-AEE7-9C6C9DB3BAEA@linux.microsoft.com> Content-Language: en-US From: Roman Kisel In-Reply-To: <1AEC6E18-313E-495F-AEE7-9C6C9DB3BAEA@linux.microsoft.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 436BF16001E X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: 8z85egxjfdnsao8zkfeug7wo673kn75g X-HE-Tag: 1721676885-995116 X-HE-Meta: U2FsdGVkX1/f9NxQqZoCCmaxRADKwubWEhd7Z230x3HtKn9ntHrVLkuQ6oQrsOEPpmcIA8z/aiW/q3C0AEojeeNtBCcizpWeT32/FxwtwZlZii1FN4nfVvPAvkM2Tu5NCSyv76ok0iC5BBeuH87gmRemQAkak+daLy2+mYEawPad6+DmObhz90xPL+gVeftRO2VV6uLvmd/RPLpcWMbehO9oF/BsaokySrwPVQUe3/C5hdcYPtFN9OsdjCaBEY3Y8IVejNYDdll0e7t+iU4A/IsW40ZY9SpHPSRh2x3eGFk8/HldoeFyurD1eelSXdk5sJB1SQSNJzj0gHcarYZKWHPtRx3rc+ZcfbbQB3NXEMUxt4DA5isFVN+aLarl1W4wPSr4QUN88UCDJ7DIBm54OZf9W+YZEqF8s8kX/B45HBSktMSdFgfOciv80rse8ErovUfy/QiNT5Poki+tAVL2YzAZgR06RO/k1QdnXXr4QY6kgukoYE7Om6pax5prdZ8H+7Curl4H79JoB89ITbUagypLeL4ZygsrTpm2EyJ/j12gN+9kqg+WKaM+WYtV5jHdcufwSiHMemGFd6y65OWuTu/TjcGV907sEbq5qm3VcgC87JBi+rYg/GeTgP1HWHU6VUt2xxooGZ5vfcEf3nZ9BSAt62NmdQFL5FskH0K6jA1Z/rcRywDu6dN3ctFynF9u/aOT1riMkuMJiPjKiM565lB7eIlJumDzEFEDgiinmQr5ieJOgyjG1DCG4+eP2x6TSuxlt8a/1dpySZBAZ8Jzdmea66wx9peXMRn1wUyAl093WICJqekU1ntO8dSEyP3Y50i/2lICw2xjvN7fnvUGF2gyrn8F4VFsvnRBVzjk8J8CGAVYljS0z4LODjN3ljNqNAQVhN1HWUFyjcQ1114Wxwo4G6yfo5mmuX4EZ7IRhRO0svvD+fbh+lTyGZ+8i+FD4inSwkBvvzCIIzOS4d4 w1KyL4ao qXuFPXwWNs5RJQoukAcdQ0HeBBC21zbF/WVD6MEtztCBOhg9K4rj1tSTXmgb/th7rPVBJBz+bkIHHHAXX8R1hH5ms1+CSeDuD+T7rgQJ2WgU6USspnflwTcYUyWTngaWp4MeN3wrTpI0B7+yqqu+bjFV5JWEIuis99V4DGgLNeKekkv4SGItCmoRw+DUpWGlu/6k/yJmONeW7Ba3z4vURisLxyTxE1DycnOoblc1s26JjUtaMSlONyllsGvcUmVnGEUNbO83Xw3B5BXJRccV8KEaULOHc+gxXqYLHibKueGHWBwfLeqwhv6ej/4eN+fhpPuTC43QP2FeJysNWVwJOi4Ujjg== 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 7/19/2024 10:33 AM, Allen Pais wrote: > > >> On Jul 18, 2024, at 11:27 AM, Roman Kisel >> wrote: >> >> The coredump code does not log the process ID and the comm >> consistently, logs unescaped comm when it does log it, and >> does not always use the ratelimited logging. That makes it >> harder to analyze logs and puts the system at the risk of >> spamming the system log incase something crashes many times >> over and over again. >> >> Fix that by logging TGID and comm (escaped) consistently and >> using the ratelimited logging always. >> >> Signed-off-by: Roman Kisel > > LGTM. > > Tested-by: Allen Pais > Allen, thank you for your help! > > Thanks. > > >> --- >> fs/coredump.c            | 43 +++++++++++++++------------------------- >> include/linux/coredump.h | 22 ++++++++++++++++++++ >> 2 files changed, 38 insertions(+), 27 deletions(-) >> >> diff --git a/fs/coredump.c b/fs/coredump.c >> index a57a06b80f57..19d3343b93c6 100644 >> --- a/fs/coredump.c >> +++ b/fs/coredump.c >> @@ -586,8 +586,7 @@ void do_coredump(const kernel_siginfo_t *siginfo) >> struct subprocess_info *sub_info; >> >> if (ispipe < 0) { >> -printk(KERN_WARNING "format_corename failed\n"); >> -printk(KERN_WARNING "Aborting core\n"); >> +coredump_report_failure("format_corename failed, aborting core"); >> goto fail_unlock; >> } >> >> @@ -607,27 +606,21 @@ void do_coredump(const kernel_siginfo_t *siginfo) >> * right pid if a thread in a multi-threaded >> * core_pattern process dies. >> */ >> -printk(KERN_WARNING >> -"Process %d(%s) has RLIMIT_CORE set to 1\n", >> -task_tgid_vnr(current), current->comm); >> -printk(KERN_WARNING "Aborting core\n"); >> +coredump_report_failure("RLIMIT_CORE is set to 1, aborting core"); >> goto fail_unlock; >> } >> cprm.limit = RLIM_INFINITY; >> >> dump_count = atomic_inc_return(&core_dump_count); >> if (core_pipe_limit && (core_pipe_limit < dump_count)) { >> -printk(KERN_WARNING "Pid %d(%s) over core_pipe_limit\n", >> -      task_tgid_vnr(current), current->comm); >> -printk(KERN_WARNING "Skipping core dump\n"); >> +coredump_report_failure("over core_pipe_limit, skipping core dump"); >> goto fail_dropcount; >> } >> >> helper_argv = kmalloc_array(argc + 1, sizeof(*helper_argv), >>    GFP_KERNEL); >> if (!helper_argv) { >> -printk(KERN_WARNING "%s failed to allocate memory\n", >> -      __func__); >> +coredump_report_failure("%s failed to allocate memory", __func__); >> goto fail_dropcount; >> } >> for (argi = 0; argi < argc; argi++) >> @@ -644,8 +637,7 @@ void do_coredump(const kernel_siginfo_t *siginfo) >> >> kfree(helper_argv); >> if (retval) { >> -printk(KERN_INFO "Core dump to |%s pipe failed\n", >> -      cn.corename); >> +coredump_report_failure("|%s pipe failed", cn.corename); >> goto close_fail; >> } >> } else { >> @@ -658,10 +650,8 @@ void do_coredump(const kernel_siginfo_t *siginfo) >> goto fail_unlock; >> >> if (need_suid_safe && cn.corename[0] != '/') { >> -printk(KERN_WARNING "Pid %d(%s) can only dump core "\ >> -"to fully qualified path!\n", >> -task_tgid_vnr(current), current->comm); >> -printk(KERN_WARNING "Skipping core dump\n"); >> +coredump_report_failure( >> +"this process can only dump core to a fully qualified path, skipping >> core dump"); >> goto fail_unlock; >> } >> >> @@ -730,13 +720,13 @@ void do_coredump(const kernel_siginfo_t *siginfo) >> idmap = file_mnt_idmap(cprm.file); >> if (!vfsuid_eq_kuid(i_uid_into_vfsuid(idmap, inode), >>    current_fsuid())) { >> -pr_info_ratelimited("Core dump to %s aborted: cannot preserve file >> owner\n", >> -   cn.corename); >> +coredump_report_failure("Core dump to %s aborted: " >> +"cannot preserve file owner", cn.corename); >> goto close_fail; >> } >> if ((inode->i_mode & 0677) != 0600) { >> -pr_info_ratelimited("Core dump to %s aborted: cannot preserve file >> permissions\n", >> -   cn.corename); >> +coredump_report_failure("Core dump to %s aborted: " >> +"cannot preserve file permissions", cn.corename); >> goto close_fail; >> } >> if (!(cprm.file->f_mode & FMODE_CAN_WRITE)) >> @@ -757,7 +747,7 @@ void do_coredump(const kernel_siginfo_t *siginfo) >> * have this set to NULL. >> */ >> if (!cprm.file) { >> -pr_info("Core dump to |%s disabled\n", cn.corename); >> +coredump_report_failure("Core dump to |%s disabled", cn.corename); >> goto close_fail; >> } >> if (!dump_vma_snapshot(&cprm)) >> @@ -983,11 +973,10 @@ void validate_coredump_safety(void) >> { >> if (suid_dumpable == SUID_DUMP_ROOT && >>    core_pattern[0] != '/' && core_pattern[0] != '|') { >> -pr_warn( >> -"Unsafe core_pattern used with fs.suid_dumpable=2.\n" >> -"Pipe handler or fully qualified core dump path required.\n" >> -"Set kernel.core_pattern before fs.suid_dumpable.\n" >> -); >> + >> +coredump_report_failure("Unsafe core_pattern used with >> fs.suid_dumpable=2: " >> +"pipe handler or fully qualified core dump path required. " >> +"Set kernel.core_pattern before fs.suid_dumpable."); >> } >> } >> >> diff --git a/include/linux/coredump.h b/include/linux/coredump.h >> index 0904ba010341..45e598fe3476 100644 >> --- a/include/linux/coredump.h >> +++ b/include/linux/coredump.h >> @@ -43,8 +43,30 @@ extern int dump_align(struct coredump_params *cprm, >> int align); >> int dump_user_range(struct coredump_params *cprm, unsigned long start, >>    unsigned long len); >> extern void do_coredump(const kernel_siginfo_t *siginfo); >> + >> +/* >> + * Logging for the coredump code, ratelimited. >> + * The TGID and comm fields are added to the message. >> + */ >> + >> +#define __COREDUMP_PRINTK(Level, Format, ...) \ >> +do {\ >> +char comm[TASK_COMM_LEN];\ >> +\ >> +get_task_comm(comm, current);\ >> +printk_ratelimited(Level "coredump: %d(%*pE): " Format "\n",\ >> +task_tgid_vnr(current), (int)strlen(comm), comm, ##__VA_ARGS__);\ >> +} while (0)\ >> + >> +#define coredump_report(fmt, ...) __COREDUMP_PRINTK(KERN_INFO, fmt, >> ##__VA_ARGS__) >> +#define coredump_report_failure(fmt, ...) >> __COREDUMP_PRINTK(KERN_WARNING, fmt, ##__VA_ARGS__) >> + >> #else >> static inline void do_coredump(const kernel_siginfo_t *siginfo) {} >> + >> +#define coredump_report(...) >> +#define coredump_report_failure(...) >> + >> #endif >> >> #if defined(CONFIG_COREDUMP) && defined(CONFIG_SYSCTL) >> -- >> 2.45.2 > -- Thank you, Roman