Received: by 2002:a25:b794:0:0:0:0:0 with SMTP id n20csp4601813ybh; Tue, 6 Aug 2019 14:39:52 -0700 (PDT) X-Google-Smtp-Source: APXvYqwKjM4HIVjIELuy8IilMfNO3neecxOPt1tCIZ3zJ49VLP4qkPKaJ7Y/I1sM6uoSu1gzLT1P X-Received: by 2002:a17:90a:db44:: with SMTP id u4mr5214006pjx.52.1565127592651; Tue, 06 Aug 2019 14:39:52 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1565127592; cv=none; d=google.com; s=arc-20160816; b=IElXnAP1+E7i1l0o64gLMR/1WBbiwnpew2l431Im8D/3FJBXWUL6tcbFgUXuDgBQH8 0DV+26obkXLLv+zqRFVoxqNUJKOFYF610y9LO1NtOvNFgprbBLq3WVmxrtAhOE6Es37A QvRHicF/OB2sOw2XvLOSNVfKite1+GZkmsefz3pPZNNVSsFF1GP44T/CvrbuCRZM6+yM DerAMBgHk/HpPcYKexqJL6oCYf1ZVztl5xVEcy1ZZabLZbEAUaER8Ftwfe5asdXiBWfk 22bxyGSys8ND8otDQ95Xt8vBU9yXhQ0QfdUJvpnWPo28Bye0JhrKGxP5Ec2soMAMUBl/ GYQA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=PkS9DicYlTOENCeVZoPbGFlxHBrOxb8a4jhwQ3slnLw=; b=QyWClAuKJkE6drFzKBhl4tQaOLiN3NAwFFuRrMmC/bX+/87IvvRk5fbs6sB91N96Ft KbAhvHsw8ietp8jwqS9k0ciPSsPVfkXgcxsfD+MYQMfEUFIktIRQS5IQEsHifnD7cj3+ t37iEeVt+6lP07IGaVrQjrNiqUZcWq9D6OJJmk7WWj+yocg1L6igOD30FCE/0tKWMKYf iIsfd5Qj+9Js9nCH1FVoEpCxcvFcCoALKdS8yhkCKrmqCz0VkXptTroSUZt1Yc0kxkm5 J18eBNt7/nj9Z6PxmIEZrm79RjH+onKZB3Wpl6DMq+s5W+Qfti7mXrsFD0LEAwN5ltdr MbWA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=BnLryYLR; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 26si49881219pgs.520.2019.08.06.14.39.37; Tue, 06 Aug 2019 14:39:52 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=BnLryYLR; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729710AbfHFViT (ORCPT + 99 others); Tue, 6 Aug 2019 17:38:19 -0400 Received: from mail.kernel.org ([198.145.29.99]:55918 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728558AbfHFViN (ORCPT ); Tue, 6 Aug 2019 17:38:13 -0400 Received: from sasha-vm.mshome.net (c-73-47-72-35.hsd1.nh.comcast.net [73.47.72.35]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id D506320C01; Tue, 6 Aug 2019 21:38:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1565127492; bh=XjzyOiIy4mUb178EFcZRr/KkuDcevmWEOjos0b5tJz0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=BnLryYLRznA3dwA5W0wGuDJcl8RHe3n+XauzrD6WXLasogX0SQDjIBumBmWLnKYM1 jHCePdXaHsKP0lwtF5s7RKwKU2Enm5034h0l2mgJMKkLsLpFqRzmQLrO9kAT5+pp8V WJxmuLzCS8+pJLJSy1juGWSBvznGVqX5iT2GD/mI= From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Paul Wise , Jakub Wilk , Neil Horman , Andrew Morton , Linus Torvalds , Sasha Levin , linux-fsdevel@vger.kernel.org Subject: [PATCH AUTOSEL 4.4 13/14] coredump: split pipe command whitespace before expanding template Date: Tue, 6 Aug 2019 17:37:47 -0400 Message-Id: <20190806213749.20689-13-sashal@kernel.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190806213749.20689-1-sashal@kernel.org> References: <20190806213749.20689-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Paul Wise [ Upstream commit 315c69261dd3fa12dbc830d4fa00d1fad98d3b03 ] Save the offsets of the start of each argument to avoid having to update pointers to each argument after every corename krealloc and to avoid having to duplicate the memory for the dump command. Executable names containing spaces were previously being expanded from %e or %E and then split in the middle of the filename. This is incorrect behaviour since an argument list can represent arguments with spaces. The splitting could lead to extra arguments being passed to the core dump handler that it might have interpreted as options or ignored completely. Core dump handlers that are not aware of this Linux kernel issue will be using %e or %E without considering that it may be split and so they will be vulnerable to processes with spaces in their names breaking their argument list. If their internals are otherwise well written, such as if they are written in shell but quote arguments, they will work better after this change than before. If they are not well written, then there is a slight chance of breakage depending on the details of the code but they will already be fairly broken by the split filenames. Core dump handlers that are aware of this Linux kernel issue will be placing %e or %E as the last item in their core_pattern and then aggregating all of the remaining arguments into one, separated by spaces. Alternatively they will be obtaining the filename via other methods. Both of these will be compatible with the new arrangement. A side effect from this change is that unknown template types (for example %z) result in an empty argument to the dump handler instead of the argument being dropped. This is a desired change as: It is easier for dump handlers to process empty arguments than dropped ones, especially if they are written in shell or don't pass each template item with a preceding command-line option in order to differentiate between individual template types. Most core_patterns in the wild do not use options so they can confuse different template types (especially numeric ones) if an earlier one gets dropped in old kernels. If the kernel introduces a new template type and a core_pattern uses it, the core dump handler might not expect that the argument can be dropped in old kernels. For example, this can result in security issues when %d is dropped in old kernels. This happened with the corekeeper package in Debian and resulted in the interface between corekeeper and Linux having to be rewritten to use command-line options to differentiate between template types. The core_pattern for most core dump handlers is written by the handler author who would generally not insert unknown template types so this change should be compatible with all the core dump handlers that exist. Link: http://lkml.kernel.org/r/20190528051142.24939-1-pabs3@bonedaddy.net Fixes: 74aadce98605 ("core_pattern: allow passing of arguments to user mode helper when core_pattern is a pipe") Signed-off-by: Paul Wise Reported-by: Jakub Wilk [https://bugs.debian.org/924398] Reported-by: Paul Wise [https://lore.kernel.org/linux-fsdevel/c8b7ecb8508895bf4adb62a748e2ea2c71854597.camel@bonedaddy.net/] Suggested-by: Jakub Wilk Acked-by: Neil Horman Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Signed-off-by: Sasha Levin --- fs/coredump.c | 44 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/fs/coredump.c b/fs/coredump.c index a8852293038a3..31f8f5eb43a03 100644 --- a/fs/coredump.c +++ b/fs/coredump.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -163,11 +164,13 @@ static int cn_print_exe_file(struct core_name *cn) * name into corename, which must have space for at least * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator. */ -static int format_corename(struct core_name *cn, struct coredump_params *cprm) +static int format_corename(struct core_name *cn, struct coredump_params *cprm, + size_t **argv, int *argc) { const struct cred *cred = current_cred(); const char *pat_ptr = core_pattern; int ispipe = (*pat_ptr == '|'); + bool was_space = false; int pid_in_pattern = 0; int err = 0; @@ -177,12 +180,35 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm) return -ENOMEM; cn->corename[0] = '\0'; - if (ispipe) + if (ispipe) { + int argvs = sizeof(core_pattern) / 2; + (*argv) = kmalloc_array(argvs, sizeof(**argv), GFP_KERNEL); + if (!(*argv)) + return -ENOMEM; + (*argv)[(*argc)++] = 0; ++pat_ptr; + } /* Repeat as long as we have more pattern to process and more output space */ while (*pat_ptr) { + /* + * Split on spaces before doing template expansion so that + * %e and %E don't get split if they have spaces in them + */ + if (ispipe) { + if (isspace(*pat_ptr)) { + was_space = true; + pat_ptr++; + continue; + } else if (was_space) { + was_space = false; + err = cn_printf(cn, "%c", '\0'); + if (err) + return err; + (*argv)[(*argc)++] = cn->used; + } + } if (*pat_ptr != '%') { err = cn_printf(cn, "%c", *pat_ptr++); } else { @@ -519,6 +545,8 @@ void do_coredump(const siginfo_t *siginfo) struct cred *cred; int retval = 0; int ispipe; + size_t *argv = NULL; + int argc = 0; struct files_struct *displaced; /* require nonrelative corefile path and be extra careful */ bool need_suid_safe = false; @@ -565,9 +593,10 @@ void do_coredump(const siginfo_t *siginfo) old_cred = override_creds(cred); - ispipe = format_corename(&cn, &cprm); + ispipe = format_corename(&cn, &cprm, &argv, &argc); if (ispipe) { + int argi; int dump_count; char **helper_argv; struct subprocess_info *sub_info; @@ -610,12 +639,16 @@ void do_coredump(const siginfo_t *siginfo) goto fail_dropcount; } - helper_argv = argv_split(GFP_KERNEL, cn.corename, NULL); + helper_argv = kmalloc_array(argc + 1, sizeof(*helper_argv), + GFP_KERNEL); if (!helper_argv) { printk(KERN_WARNING "%s failed to allocate memory\n", __func__); goto fail_dropcount; } + for (argi = 0; argi < argc; argi++) + helper_argv[argi] = cn.corename + argv[argi]; + helper_argv[argi] = NULL; retval = -ENOMEM; sub_info = call_usermodehelper_setup(helper_argv[0], @@ -625,7 +658,7 @@ void do_coredump(const siginfo_t *siginfo) retval = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC); - argv_free(helper_argv); + kfree(helper_argv); if (retval) { printk(KERN_INFO "Core dump to |%s pipe failed\n", cn.corename); @@ -744,6 +777,7 @@ void do_coredump(const siginfo_t *siginfo) if (ispipe) atomic_dec(&core_dump_count); fail_unlock: + kfree(argv); kfree(cn.corename); coredump_finish(mm, core_dumped); revert_creds(old_cred); -- 2.20.1