On Mon, 5 Jul 2021 17:10:19 +0200 Vladimir Divjak <vladimir.divjak@xxxxxx> wrote:
> This patch allows the coredump user mode helper process,
> if one is configured (in core_pattern),
> to perform ptrace operations on the dying process
> whose cordump it's handling.
>
> The user mode helper process is expected to do so
> before consuming the coredump data from the pipe,
> and thereby, before the dying process is reaped by kernel.
>
> Issuing a PTRACE_ATTACH request will pause the coredumping
> for that task until ptrace operation is finished.
>
> The user mode helper process is also expected to
> issue a PTRACE_CONT request to the dying process,
> when it is done ptracing it, signaling the dying process
> coredumping can be resumed.
>
> * Problem description / Rationale:
> In automotive and/or embedded environments,
> the storage capacity to store, and/or
> network capabilities to upload
> a complete core file can easily be a limiting factor,
> making offline issue analysis difficult.
>
> * Solution:
> Allow the user mode coredump helper process
> to perform ptrace on the dying process in order to obtain
> useful information such as user mode stacktrace, and
> thereby greatly improve the offline debugging possibilities
> for such environments.
>
> * Impact / Risk:
> The user mode helper process is already entrusted
> with handling the coredump data, so allowing it to read or even change
> the dying process memory should not pose an additional risk.
>
> Furthermore, this change makes coredump emission somewhat slower
> due to the additional step of iterating over the core dump helper list
> and checking if ptrace completion needs to be awaited,
> during coredump emission.
>
Seems useful.
>
> ...
>
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -41,6 +41,7 @@
> #include <linux/fs.h>
> #include <linux/path.h>
> #include <linux/timekeeping.h>
> +#include <linux/jiffies.h>
>
> #include <linux/uaccess.h>
> #include <asm/mmu_context.h>
> @@ -62,6 +63,64 @@ struct core_name {
> int used, size;
> };
>
> +DEFINE_MUTEX(cdh_mutex);
> +LIST_HEAD(cdh_list);
I think this could be static?
> + struct task_struct *task_being_dumped;
> + struct completion ptrace_done;
> + pid_t helper_pid;
> +};
>
> ...
>
> @@ -692,9 +751,14 @@ void do_coredump(const kernel_siginfo_t *siginfo)
> sub_info = call_usermodehelper_setup(helper_argv[0],
> helper_argv, NULL, GFP_KERNEL,
> umh_pipe_setup, NULL, &cprm);
> - if (sub_info)
> + if (sub_info) {
> + mutex_lock(&cdh_mutex);
> retval = call_usermodehelper_exec(sub_info,
> UMH_WAIT_EXEC);
> + if (!retval)
> + cdh_link_current_locked(sub_info->pid);
> + mutex_unlock(&cdh_mutex);
Dumb question: can the usermode helper coredump and then try to take
cdh_mutex, causing a deadlock?
> + }
>
> kfree(helper_argv);
> if (retval) {
>
> ...
>
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
It seems regrettable that the ptrace code has to be aware of the
coredump code in this fashion.
