linux-kernel - Re: [PATCH] coredump debugging: add a tracepoint to report the coredumping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <tencent_649330BFF9D93CB7540D508B05514FCEA40A@qq.com>
Date: Wed, 21 Feb 2024 23:45:58 +0800
From: Wen Yang <wenyang.linux@...mail.com>
To: Oleg Nesterov <oleg@...hat.com>, Steven Rostedt <rostedt@...dmis.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Ingo Molnar <mingo@...nel.org>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Mel Gorman <mgorman@...hsingularity.net>,
 Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] coredump debugging: add a tracepoint to report the
 coredumping



On 2024/2/20 01:00, Oleg Nesterov wrote:
> On 02/19, Steven Rostedt wrote:
>>
>> On Sat, 17 Feb 2024 11:49:24 +0100
>> Oleg Nesterov <oleg@...hat.com> wrote:
>>
>>> On 02/17, wenyang.linux@...mail.com wrote:
>>>>
>>>> From: Wen Yang <wenyang.linux@...mail.com>
>>>>
>>>> Currently coredump_task_exit() takes some time to wait for the generation
>>>> of the dump file. But if the user-space wants to receive a notification
>>>> as soon as possible it maybe inconvenient.
>>>>
>>>> Add the new trace_sched_process_coredump() into coredump_task_exit(),
>>>> this way a user-space monitor could easily wait for the exits and
>>>> potentially make some preparations in advance.
>>>
>>> Can't comment, I never know when the new tracepoint will make sense.
>>>
>>> Stupid question. Can we simply shift trace_sched_process_exit() up
>>> before coredump_task_exit() ?
>>
>> Reading the rest of the thread and looking at the code, we do have this:
>>
>> void __noreturn do_exit(long code)
>> {
>> 	struct task_struct *tsk = current;
>> 	int group_dead;
>>
>> [...]
>> 	acct_collect(code, group_dead);
>> 	if (group_dead)
>> 		tty_audit_exit();
>> 	audit_free(tsk);
>>
>> 	tsk->exit_code = code;
>> 	taskstats_exit(tsk, group_dead);
>>
>> 	exit_mm();
>>
>> 	if (group_dead)
>> 		acct_process();
>> 	trace_sched_process_exit(tsk);
>>
>> There's a lot that happens before we trigger the above event.
> 
> and a lot after.
> 
> To me the current placement of  looks absolutely
> random.
> 
>> I could
>> imagine that there are users expecting those actions to have taken place by
>> the time the event triggered. Like the "exit_mm()" call, as well as many
>> others.
>>
>> I would be leery of moving that tracepoint.
> 
> And I agree. I am always scared of every user-visible change, simply
> because it is user-visbible.
> 
> If it was not clear, I didn't try to nack this patch. I simply do not know
> how people use the tracepoints and for what. Apart from debugging.
> 
> But if we add the new one into coredump_task_exit(), then we probably want
> another one in ptrace_event(PTRACE_EVENT_EXIT) ? It too can "take some time"
> before the exiting task actually exits.
> 
> So I think this needs some discussion, and the changelog should probably say
> more.
> 
> In short: I am glad you are here, I leave this to you and Wen ;)
> 

Thank you Oleg, thank you Steven，

We could first put trace_sched_process_exit() aside and take a look at 
these three patches:

2d4bcf886e42f0f4846a3d9bdc3a90d278903a2e ("exit: Remove 
profile_task_exit & profile_munmap")

586b58cac8b4683eb58a1446fbc399de18974e40 (“exit: Move preemption fixup 
up, move blocking operations down”)

And the original: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2


Could we add a new TP similar to profile_task_exit()?

--
Best wishes,
Wen