History

coredump: split pipe command whitespace before expanding template

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.

Paul Wise committed 7 years ago

Commit 315c6921 parent ee38d941 file · +39 −5Browse files

Unified · Split

fs/coredump.c+39−5
@@ -7,6 +7,7 @@ · 6 unmodified lines
77 #include <linux/stat.h>
88 #include <linux/fcntl.h>
99 #include <linux/swap.h>
10+#include <linux/ctype.h>
1011 #include <linux/string.h>
1112 #include <linux/init.h>
1213 #include <linux/pagemap.h>
@@ -187,11 +188,13 @@ put_exe_file: · 174 unmodified lines
187188 * name into corename, which must have space for at least
188189 * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator.
189190 */
190−static int format_corename(struct core_name *cn, struct coredump_params *cprm)
191+static int format_corename(struct core_name *cn, struct coredump_params *cprm,
192+ size_t **argv, int *argc)
191193 {
192194 const struct cred *cred = current_cred();
193195 const char *pat_ptr = core_pattern;
194196 int ispipe = (*pat_ptr == '|');
197+ bool was_space = false;
195198 int pid_in_pattern = 0;
196199 int err = 0;
197200
@@ -201,12 +204,35 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm) · 3 unmodified lines
201204 return -ENOMEM;
202205 cn->corename[0] = '\0';
203206
204− if (ispipe)
207+ if (ispipe) {
208+ int argvs = sizeof(core_pattern) / 2;
209+ (*argv) = kmalloc_array(argvs, sizeof(**argv), GFP_KERNEL);
210+ if (!(*argv))
211+ return -ENOMEM;
212+ (*argv)[(*argc)++] = 0;
205213 ++pat_ptr;
214+ }
206215
207216 /* Repeat as long as we have more pattern to process and more output
208217 space */
209218 while (*pat_ptr) {
219+ /*
220+ * Split on spaces before doing template expansion so that
221+ * %e and %E don't get split if they have spaces in them
222+ */
223+ if (ispipe) {
224+ if (isspace(*pat_ptr)) {
225+ was_space = true;
226+ pat_ptr++;
227+ continue;
228+ } else if (was_space) {
229+ was_space = false;
230+ err = cn_printf(cn, "%c", '\0');
231+ if (err)
232+ return err;
233+ (*argv)[(*argc)++] = cn->used;
234+ }
235+ }
210236 if (*pat_ptr != '%') {
211237 err = cn_printf(cn, "%c", *pat_ptr++);
212238 } else {
@@ -546,6 +572,8 @@ void do_coredump(const kernel_siginfo_t *siginfo) · 333 unmodified lines
546572 struct cred *cred;
547573 int retval = 0;
548574 int ispipe;
575+ size_t *argv = NULL;
576+ int argc = 0;
549577 struct files_struct *displaced;
550578 /* require nonrelative corefile path and be extra careful */
551579 bool need_suid_safe = false;
@@ -592,9 +620,10 @@ void do_coredump(const kernel_siginfo_t *siginfo) · 40 unmodified lines
592620
593621 old_cred = override_creds(cred);
594622
595− ispipe = format_corename(&cn, &cprm);
623+ ispipe = format_corename(&cn, &cprm, &argv, &argc);
596624
597625 if (ispipe) {
626+ int argi;
598627 int dump_count;
599628 char **helper_argv;
600629 struct subprocess_info *sub_info;
@@ -637,12 +666,16 @@ void do_coredump(const kernel_siginfo_t *siginfo) · 36 unmodified lines
637666 goto fail_dropcount;
638667 }
639668
640− helper_argv = argv_split(GFP_KERNEL, cn.corename, NULL);
669+ helper_argv = kmalloc_array(argc + 1, sizeof(*helper_argv),
670+ GFP_KERNEL);
641671 if (!helper_argv) {
642672 printk(KERN_WARNING "%s failed to allocate memory\n",
643673 __func__);
644674 goto fail_dropcount;
645675 }
676+ for (argi = 0; argi < argc; argi++)
677+ helper_argv[argi] = cn.corename + argv[argi];
678+ helper_argv[argi] = NULL;
646679
647680 retval = -ENOMEM;
648681 sub_info = call_usermodehelper_setup(helper_argv[0],
@@ -652,7 +685,7 @@ void do_coredump(const kernel_siginfo_t *siginfo) · 3 unmodified lines
652685 retval = call_usermodehelper_exec(sub_info,
653686 UMH_WAIT_EXEC);
654687
655− argv_free(helper_argv);
688+ kfree(helper_argv);
656689 if (retval) {
657690 printk(KERN_INFO "Core dump to |%s pipe failed\n",
658691 cn.corename);
@@ -766,6 +799,7 @@ fail_dropcount: · 107 unmodified lines
766799 if (ispipe)
767800 atomic_dec(&core_dump_count);
768801 fail_unlock:
802+ kfree(argv);
769803 kfree(cn.corename);
770804 coredump_finish(mm, core_dumped);
771805 revert_creds(old_cred);

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t