[PATCH 1/1] connector: Added coredumping event to the process connector

[PATCH 1/1] connector: Added coredumping event to the process connector

[Jesper Derehag]

Process connector can now also detect coredumping events. Could be used for instance by process-managers that want to get notified immediatly at process crash instead of waiting for exit.

Signed-off-by: Jesper Derehag <jderehag@hotmail.com>
---
 drivers/connector/cn_proc.c  |   27 +++++++++++++++++++++++++++
 include/linux/cn_proc.h      |    4 ++++
 include/uapi/linux/cn_proc.h |    6 ++++--
 kernel/signal.c              |    2 ++
 4 filer ändrade, 37 tillägg(+), 2 borttagningar(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 1110478..325f11c 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -232,6 +232,33 @@ void proc_comm_connector(struct task_struct *task)
 	cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
 }
 
+void proc_coredump_connector(struct task_struct *task)
+{
+	struct cn_msg *msg;
+	struct proc_event *ev;
+	__u8 buffer[CN_PROC_MSG_SIZE];
+	struct timespec ts;
+
+	if (atomic_read(&proc_event_num_listeners) < 1)
+		return;
+
+	msg = (struct cn_msg *)buffer;
+	ev = (struct proc_event *)msg->data;
+	get_seq(&msg->seq, &ev->cpu);
+	ktime_get_ts(&ts); /* get high res monotonic timestamp */
+	put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
+	ev->what = PROC_EVENT_COREDUMP;
+	ev->event_data.exit.process_pid = task->pid;
+	ev->event_data.exit.process_tgid = task->tgid;
+	ev->event_data.exit.exit_code = task->exit_code;
+	ev->event_data.exit.exit_signal = task->exit_signal;
+
+	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
+	msg->ack = 0; /* not used */
+	msg->len = sizeof(*ev);
+	cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+}
+
 void proc_exit_connector(struct task_struct *task)
 {
 	struct cn_msg *msg;
diff --git a/include/linux/cn_proc.h b/include/linux/cn_proc.h
index 2c1bc1e..1d5b02a 100644
--- a/include/linux/cn_proc.h
+++ b/include/linux/cn_proc.h
@@ -26,6 +26,7 @@ void proc_id_connector(struct task_struct *task, int which_id);
 void proc_sid_connector(struct task_struct *task);
 void proc_ptrace_connector(struct task_struct *task, int which_id);
 void proc_comm_connector(struct task_struct *task);
+void proc_coredump_connector(struct task_struct *task);
 void proc_exit_connector(struct task_struct *task);
 #else
 static inline void proc_fork_connector(struct task_struct *task)
@@ -48,6 +49,9 @@ static inline void proc_ptrace_connector(struct task_struct *task,
 					 int ptrace_id)
 {}
 
+static inline void proc_coredump_connector(struct task_struct *task)
+{}
+
 static inline void proc_exit_connector(struct task_struct *task)
 {}
 #endif	/* CONFIG_PROC_EVENTS */
diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h
index 0d7b499..a46caf1 100644
--- a/include/uapi/linux/cn_proc.h
+++ b/include/uapi/linux/cn_proc.h
@@ -55,8 +55,10 @@ struct proc_event {
 		PROC_EVENT_SID  = 0x00000080,
 		PROC_EVENT_PTRACE = 0x00000100,
 		PROC_EVENT_COMM = 0x00000200,
-		/* "next" should be 0x00000400 */
-		/* "last" is the last process event: exit */
+		/* "next" should be 0x00000400 */
+		/* "last" is the last process event: exit,
+		 * while "next to last" is coredumping event */
+		PROC_EVENT_COREDUMP = 0x40000000,
 		PROC_EVENT_EXIT = 0x80000000
 	} what;
 	__u32 cpu;
diff --git a/kernel/signal.c b/kernel/signal.c
index d63c79e..e591731 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -32,6 +32,7 @@
 #include <linux/user_namespace.h>
 #include <linux/uprobes.h>
 #include <linux/compat.h>
+#include <linux/cn_proc.h>
 #define CREATE_TRACE_POINTS
 #include <trace/events/signal.h>
 
@@ -2347,6 +2348,7 @@ relock:
 		if (sig_kernel_coredump(signr)) {
 			if (print_fatal_signals)
 				print_fatal_signal(info->si_signo);
+			proc_coredump_connector(current);
 			/*
 			 * If it was able to dump core, this kills all
 			 * other threads in the group and synchronizes with
-- 
1.7.10.4

Re: [PATCH 1/1] connector: Added coredumping event to the process connector

[Hannes Frederic Sowa]

On Sat, Mar 16, 2013 at 11:50:50AM +0100, Jesper Derehag wrote:
> +	ev->event_data.exit.exit_code = task->exit_code;
> +	ev->event_data.exit.exit_signal = task->exit_signal;

Do these already contain meaningful values?

Re: [PATCH 1/1] connector: Added coredumping event to the process connector

[Hannes Frederic Sowa]

On Sat, Mar 16, 2013 at 05:57:20PM +0000, Jesper Derehag wrote:
> 
> 
> > Date: Sat, 16 Mar 2013 18:03:48 +0100
> > From: hannes@stressinduktion.org
> > To: jderehag@hotmail.com
> > CC: zbr@ioremap.net; netdev@vger.kernel.org
> > Subject: Re: [PATCH 1/1] connector: Added coredumping event to the process connector
> > 
> > On Sat, Mar 16, 2013 at 11:50:50AM +0100, Jesper Derehag wrote:
> > > +	ev->event_data.exit.exit_code = task->exit_code;
> > > +	ev->event_data.exit.exit_signal = task->exit_signal;
> > 
> > Do these already contain meaningful values?
> > 
> 
> I have to admit that they dont.And you are correct, I should add a new event struct specific for the coredump event instead of piggybacking on the exit struct.Will re-submit a patch..

Hm, I am still unsure if such a patch is needed. Couldn't you test for
coredump by inspecting exit_code on PROC_EVENT_EXIT?

RE: [PATCH 1/1] connector: Added coredumping event to the process connector

[Jesper Derehag]

----------------------------------------
> Date: Sat, 16 Mar 2013 19:40:36 +0100
> From: hannes@stressinduktion.org
> To: jderehag@hotmail.com
> CC: zbr@ioremap.net; netdev@vger.kernel.org
> Subject: Re: [PATCH 1/1] connector: Added coredumping event to the process connector
>
> On Sat, Mar 16, 2013 at 05:57:20PM +0000, Jesper Derehag wrote:
> >
> >
> > > Date: Sat, 16 Mar 2013 18:03:48 +0100
> > > From: hannes@stressinduktion.org
> > > To: jderehag@hotmail.com
> > > CC: zbr@ioremap.net; netdev@vger.kernel.org
> > > Subject: Re: [PATCH 1/1] connector: Added coredumping event to the process connector
> > >
> > > On Sat, Mar 16, 2013 at 11:50:50AM +0100, Jesper Derehag wrote:
> > > > + ev->event_data.exit.exit_code = task->exit_code;
> > > > + ev->event_data.exit.exit_signal = task->exit_signal;
> > >
> > > Do these already contain meaningful values?
> > >
> >
> > I have to admit that they dont.And you are correct, I should add a new event struct specific for the coredump event instead of piggybacking on the exit struct.Will re-submit a patch..
>
> Hm, I am still unsure if such a patch is needed. Couldn't you test for
> coredump by inspecting exit_code on PROC_EVENT_EXIT?

*** resubmitted message due to it got dropped by vger.kernel.org ***

 Well, what this patch adds I think is more a question of timing. 
 As an example, say you want to quickly detect process failures. In that case if we would only have the EXIT event, that would mean that we get notified after the dump is done, which could take minutes depending on how large the dump is. 
If we instead watch for both EXIT & COREDUMP events, it would mean that we would quickly catch any failing process, regardless of if its actually starting to coredump or if its exited for some other reason. 		 	   		  

RE: [PATCH 1/1] connector: Added coredumping event to the process connector

[Jesper Derehag]

> From: jderehag@hotmail.com
> To: netdev@vger.kernel.org
> Subject: RE: [PATCH 1/1] connector: Added coredumping event to the process connector
> Date: Sat, 16 Mar 2013 19:08:03 +0000
> 
> ----------------------------------------
> > Date: Sat, 16 Mar 2013 19:40:36 +0100
> > From: hannes@stressinduktion.org
> > To: jderehag@hotmail.com
> > CC: zbr@ioremap.net; netdev@vger.kernel.org
> > Subject: Re: [PATCH 1/1] connector: Added coredumping event to the process connector
> >
> > On Sat, Mar 16, 2013 at 05:57:20PM +0000, Jesper Derehag wrote:
> > >
> > >
> > > > Date: Sat, 16 Mar 2013 18:03:48 +0100
> > > > From: hannes@stressinduktion.org
> > > > To: jderehag@hotmail.com
> > > > CC: zbr@ioremap.net; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH 1/1] connector: Added coredumping event to the process connector
> > > >
> > > > On Sat, Mar 16, 2013 at 11:50:50AM +0100, Jesper Derehag wrote:
> > > > > + ev->event_data.exit.exit_code = task->exit_code;
> > > > > + ev->event_data.exit.exit_signal = task->exit_signal;
> > > >
> > > > Do these already contain meaningful values?
> > > >
> > >
> > > I have to admit that they dont.And you are correct, I should add a new event struct specific for the coredump event instead of piggybacking on the exit struct.Will re-submit a patch..
> >
> > Hm, I am still unsure if such a patch is needed. Couldn't you test for
> > coredump by inspecting exit_code on PROC_EVENT_EXIT?
> 
> *** resubmitted message due to it got dropped by vger.kernel.org ***
> 
>  Well, what this patch adds I think is more a question of timing. 
>  As an example, say you want to quickly detect process failures. In that case if we would only have the EXIT event, that would mean that we get notified after the dump is done, which could take minutes depending on how large the dump is. 
> If we instead watch for both EXIT & COREDUMP events, it would mean that we would quickly catch any failing process, regardless of if its actually starting to coredump or if its exited for some other reason. 


Any other comments on this before I send a v2 patch with the exit vs the coredump event struct change?

Regards,
Jesper 		 	   		  

Re: [PATCH 1/1] connector: Added coredumping event to the process connector

[Hannes Frederic Sowa]

On Tue, Mar 19, 2013 at 07:32:10AM +0000, Jesper Derehag wrote:
> > From: jderehag@hotmail.com
> > To: netdev@vger.kernel.org
> > Subject: RE: [PATCH 1/1] connector: Added coredumping event to the process connector
> > Date: Sat, 16 Mar 2013 19:08:03 +0000
> > 
> > ----------------------------------------
> > > Date: Sat, 16 Mar 2013 19:40:36 +0100
> > > From: hannes@stressinduktion.org
> > > To: jderehag@hotmail.com
> > > CC: zbr@ioremap.net; netdev@vger.kernel.org
> > > Subject: Re: [PATCH 1/1] connector: Added coredumping event to the process connector
> > >
> > > On Sat, Mar 16, 2013 at 05:57:20PM +0000, Jesper Derehag wrote:
> > > >
> > > >
> > > > > Date: Sat, 16 Mar 2013 18:03:48 +0100
> > > > > From: hannes@stressinduktion.org
> > > > > To: jderehag@hotmail.com
> > > > > CC: zbr@ioremap.net; netdev@vger.kernel.org
> > > > > Subject: Re: [PATCH 1/1] connector: Added coredumping event to the process connector
> > > > >
> > > > > On Sat, Mar 16, 2013 at 11:50:50AM +0100, Jesper Derehag wrote:
> > > > > > + ev->event_data.exit.exit_code = task->exit_code;
> > > > > > + ev->event_data.exit.exit_signal = task->exit_signal;
> > > > >
> > > > > Do these already contain meaningful values?
> > > > >
> > > >
> > > > I have to admit that they dont.And you are correct, I should add a new event struct specific for the coredump event instead of piggybacking on the exit struct.Will re-submit a patch..
> > >
> > > Hm, I am still unsure if such a patch is needed. Couldn't you test for
> > > coredump by inspecting exit_code on PROC_EVENT_EXIT?
> > 
> > *** resubmitted message due to it got dropped by vger.kernel.org ***
> > 
> >  Well, what this patch adds I think is more a question of timing. 
> >  As an example, say you want to quickly detect process failures. In that case if we would only have the EXIT event, that would mean that we get notified after the dump is done, which could take minutes depending on how large the dump is. 
> > If we instead watch for both EXIT & COREDUMP events, it would mean that we would quickly catch any failing process, regardless of if its actually starting to coredump or if its exited for some other reason. 		 	   		  
> 
> 
> Any other comments on this before I send a v2 patch with the exit vs the coredump event struct change?

I would say just go on and submit the patch. Perhaps you can Cc someone
looking after the change in signal.c (maybe lkml). I just checked that
you don't hold any spinlocks while doing the netlink send.

RE: [PATCH 1/1] connector: Added coredumping event to the process connector

[Jesper Derehag]

----------------------------------------
> Date: Tue, 19 Mar 2013 18:09:32 +0100
> From: hannes@stressinduktion.org
> To: jderehag@hotmail.com
> CC: netdev@vger.kernel.org; zbr@ioremap.net
> Subject: Re: [PATCH 1/1] connector: Added coredumping event to the process connector
>
> On Tue, Mar 19, 2013 at 07:32:10AM +0000, Jesper Derehag wrote:
> > > From: jderehag@hotmail.com
> > > To: netdev@vger.kernel.org
> > > Subject: RE: [PATCH 1/1] connector: Added coredumping event to the process connector
> > > Date: Sat, 16 Mar 2013 19:08:03 +0000
> > >
> > > ----------------------------------------
> > > > Date: Sat, 16 Mar 2013 19:40:36 +0100
> > > > From: hannes@stressinduktion.org
> > > > To: jderehag@hotmail.com
> > > > CC: zbr@ioremap.net; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH 1/1] connector: Added coredumping event to the process connector
> > > >
> > > > On Sat, Mar 16, 2013 at 05:57:20PM +0000, Jesper Derehag wrote:
> > > > >
> > > > >
> > > > > > Date: Sat, 16 Mar 2013 18:03:48 +0100
> > > > > > From: hannes@stressinduktion.org
> > > > > > To: jderehag@hotmail.com
> > > > > > CC: zbr@ioremap.net; netdev@vger.kernel.org
> > > > > > Subject: Re: [PATCH 1/1] connector: Added coredumping event to the process connector
> > > > > >
> > > > > > On Sat, Mar 16, 2013 at 11:50:50AM +0100, Jesper Derehag wrote:
> > > > > > > + ev->event_data.exit.exit_code = task->exit_code;
> > > > > > > + ev->event_data.exit.exit_signal = task->exit_signal;
> > > > > >
> > > > > > Do these already contain meaningful values?
> > > > > >
> > > > >
> > > > > I have to admit that they dont.And you are correct, I should add a new event struct specific for the coredump event instead of piggybacking on the exit struct.Will re-submit a patch..
> > > >
> > > > Hm, I am still unsure if such a patch is needed. Couldn't you test for
> > > > coredump by inspecting exit_code on PROC_EVENT_EXIT?
> > >
> > > *** resubmitted message due to it got dropped by vger.kernel.org ***
> > >
> > > Well, what this patch adds I think is more a question of timing.
> > > As an example, say you want to quickly detect process failures. In that case if we would only have the EXIT event, that would mean that we get notified after the dump is done, which could take minutes depending on how large the dump is.
> > > If we instead watch for both EXIT & COREDUMP events, it would mean that we would quickly catch any failing process, regardless of if its actually starting to coredump or if its exited for some other reason.
> >
> >
> > Any other comments on this before I send a v2 patch with the exit vs the coredump event struct change?
>
> I would say just go on and submit the patch. Perhaps you can Cc someone
> looking after the change in signal.c (maybe lkml). I just checked that
> you don't hold any spinlocks while doing the netlink send.
>

Great.
Just submitted the v2 patch (and cc:ing lkml), thanks for your help! Much appreciated..