public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2.6.9] fork: add a hook in do_fork()
@ 2004-11-23  6:03 Guillaume Thouvenin
  2004-11-23  8:06 ` Greg KH
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Guillaume Thouvenin @ 2004-11-23  6:03 UTC (permalink / raw)
  To: lkml; +Cc: Andrew Morton, guillaume.thouvenin

Hello,

   For a module, I need to execute a function when a fork occurs. My
solution is to add a pointer to a function (called fork_hook) in the
do_fork() and if this pointer isn't NULL, I call the function. To update
the pointer to the function I export a symbol (called trace_fork) that
defines another function with two parameters (the hook and an
identifier). This function provides a simple mechanism to manage access
to the fork_hook variable.

   I don't know if this solution is good but it's easy to implement and
it just does the trick. I made some tests and it doesn't impact the
performance of the Linux kernel. 

   I'd like to have your comment about this patch. Is it useful and is
it needed by someone else than me? 

Any comments are welcome,

Guillaume

Signed-Off-By: Guillaume Thouvenin <guillaume.thouvenin@bull.net>

--- kernel/fork.c.orig	2004-10-19 08:41:53.000000000 +0200
+++ kernel/fork.c	2004-11-22 14:45:15.700858800 +0100
@@ -60,6 +60,51 @@ rwlock_t tasklist_lock __cacheline_align
 
 EXPORT_SYMBOL(tasklist_lock);
 
+/* 
+ * fork_hook is a pointer to a function to call when forking if it 
+ * isn't NULL.  
+ */
+void (*fork_hook) (int, int) = NULL;
+
+/**
+ * trace_fork: call a given external function when forking
+ * @func: function to call
+ * @id: identifier of fork_hook's owner
+ *
+ * This function sets the fork_hook and makes some sanity checks.
+ * Function returns 0 on success, -1 on failure (ie hook is 
+ * already used).
+ */
+int trace_fork(void (*func) (int, int), int id)
+{
+	/* 
+	 * fork_hook_id is used as a lock to protect the use of 
+	 * fork_hook variable. A module can set the fork_hook 
+	 * variable if it's not already used (fork_hook_id == 0)
+	 * or if it has the corresponding fork_hook_id. 
+	 * We use a static variable to keep its last value.
+	 */
+	static int fork_hook_id = 0;
+
+	/* We can set the hook if it's not already used */
+	if ((func != NULL) && (fork_hook_id == 0)) {
+		fork_hook = func;
+		fork_hook_id = id;
+		return 0;
+	}
+
+	/* We can remove the hook if we are the owner */
+	if ((func == NULL) && (fork_hook_id == id)) {
+		fork_hook = NULL;
+		fork_hook_id = 0;
+		return 0;
+	}
+
+	/* Request can not be satisfied */
+	return -1;
+}
+EXPORT_SYMBOL(trace_fork);
+
 int nr_processes(void)
 {
 	int cpu;
@@ -1281,6 +1326,10 @@ long do_fork(unsigned long clone_flags,
 		free_pidmap(pid);
 		pid = PTR_ERR(p);
 	}
+
+	if (fork_hook != NULL)
+		fork_hook(current->pid, pid);
+
 	return pid;
 }



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23  6:03 [PATCH 2.6.9] fork: add a hook in do_fork() Guillaume Thouvenin
@ 2004-11-23  8:06 ` Greg KH
  2004-11-23  8:56   ` Guillaume Thouvenin
  2004-11-23 14:14   ` Guillaume Thouvenin
  2004-11-23  9:03 ` Christoph Hellwig
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Greg KH @ 2004-11-23  8:06 UTC (permalink / raw)
  To: Guillaume Thouvenin; +Cc: lkml, Andrew Morton

On Tue, Nov 23, 2004 at 07:03:17AM +0100, Guillaume Thouvenin wrote:
> 
>    I don't know if this solution is good but it's easy to implement and
> it just does the trick. I made some tests and it doesn't impact the
> performance of the Linux kernel. 

What's wrong with the LSM hook already available for you to use for this
function?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23  8:06 ` Greg KH
@ 2004-11-23  8:56   ` Guillaume Thouvenin
  2004-11-23 10:09     ` Guillaume Thouvenin
  2004-11-23 14:14   ` Guillaume Thouvenin
  1 sibling, 1 reply; 13+ messages in thread
From: Guillaume Thouvenin @ 2004-11-23  8:56 UTC (permalink / raw)
  To: Greg KH; +Cc: lkml, Andrew Morton, guillaume.thouvenin

On Tue, 2004-11-23 at 00:06 -0800, Greg KH wrote:
> On Tue, Nov 23, 2004 at 07:03:17AM +0100, Guillaume Thouvenin wrote:
> > 
> >    I don't know if this solution is good but it's easy to implement and
> > it just does the trick. I made some tests and it doesn't impact the
> > performance of the Linux kernel. 
> 
> What's wrong with the LSM hook already available for you to use for this
> function?
> 
> thanks,

In fact I don't see how to use LSM hook to get information like PID of
the parent and the child. As far as I understand LSM hook, I can
register my own pointer to security_operations and like this, I can add
an hook in fork through the security_task_create() right? So my function
will be called each time a new process will be created but the only
parameter passed to the function is the clone_flags.

Is it possible to get the parent PID and the child PID which are
involved when the fork occurred?

Thank you very much for your help,
Guillaume


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23  6:03 [PATCH 2.6.9] fork: add a hook in do_fork() Guillaume Thouvenin
  2004-11-23  8:06 ` Greg KH
@ 2004-11-23  9:03 ` Christoph Hellwig
  2004-11-23  9:33   ` Guillaume Thouvenin
  2004-11-23  9:59 ` Hua Zhong
  2004-11-23 13:55 ` Jan Engelhardt
  3 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2004-11-23  9:03 UTC (permalink / raw)
  To: Guillaume Thouvenin; +Cc: lkml, Andrew Morton

On Tue, Nov 23, 2004 at 07:03:17AM +0100, Guillaume Thouvenin wrote:
> Hello,
> 
>    For a module, I need to execute a function when a fork occurs. My
> solution is to add a pointer to a function (called fork_hook) in the
> do_fork() and if this pointer isn't NULL, I call the function. To update
> the pointer to the function I export a symbol (called trace_fork) that
> defines another function with two parameters (the hook and an
> identifier). This function provides a simple mechanism to manage access
> to the fork_hook variable.
> 
>    I don't know if this solution is good but it's easy to implement and
> it just does the trick. I made some tests and it doesn't impact the
> performance of the Linux kernel. 
> 
>    I'd like to have your comment about this patch. Is it useful and is
> it needed by someone else than me? 

Use SGI's PAGG patches if you want such hooks.  Also this is clearly
a _GPL export.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23  9:03 ` Christoph Hellwig
@ 2004-11-23  9:33   ` Guillaume Thouvenin
  2004-11-23 19:31     ` Jay Lan
  0 siblings, 1 reply; 13+ messages in thread
From: Guillaume Thouvenin @ 2004-11-23  9:33 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: lkml, Andrew Morton, Jay Lan

On Tue, 2004-11-23 at 09:03 +0000, Christoph Hellwig wrote:
> On Tue, Nov 23, 2004 at 07:03:17AM +0100, Guillaume Thouvenin wrote:
> > 
> >    For a module, I need to execute a function when a fork occurs. My
> > solution is to add a pointer to a function (called fork_hook) in the
> > do_fork() and if this pointer isn't NULL, I call the function. To update
> > the pointer to the function I export a symbol (called trace_fork) that
> > defines another function with two parameters (the hook and an
> > identifier). This function provides a simple mechanism to manage access
> > to the fork_hook variable.
> > 
> Use SGI's PAGG patches if you want such hooks.  Also this is clearly
> a _GPL export.

PAGG is more intrusive than my patch due to the management of groups of
processes. This hook in the fork allows me to provide a solution to do
per-group accounting with a module. If PAGG is added in the Linux Kernel
Tree it could be the solution, you are right. 

Guillaume 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23  6:03 [PATCH 2.6.9] fork: add a hook in do_fork() Guillaume Thouvenin
  2004-11-23  8:06 ` Greg KH
  2004-11-23  9:03 ` Christoph Hellwig
@ 2004-11-23  9:59 ` Hua Zhong
  2004-11-23 10:14   ` Guillaume Thouvenin
  2004-11-23 13:55 ` Jan Engelhardt
  3 siblings, 1 reply; 13+ messages in thread
From: Hua Zhong @ 2004-11-23  9:59 UTC (permalink / raw)
  To: 'Guillaume Thouvenin', 'lkml'; +Cc: 'Andrew Morton'

> +	static int fork_hook_id = 0;
> +
> +	/* We can set the hook if it's not already used */
> +	if ((func != NULL) && (fork_hook_id == 0)) {
> +		fork_hook = func;
> +		fork_hook_id = id;
> +		return 0;
> +	}

What happens if two modules are calling the same function at the same time?

> +
> +	if (fork_hook != NULL)
> +		fork_hook(current->pid, pid);
> +
>  	return pid;

What happens if the module is unloaded between the test and the call to
fork_hook?

Hua


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23  8:56   ` Guillaume Thouvenin
@ 2004-11-23 10:09     ` Guillaume Thouvenin
  0 siblings, 0 replies; 13+ messages in thread
From: Guillaume Thouvenin @ 2004-11-23 10:09 UTC (permalink / raw)
  To: Greg KH; +Cc: lkml, Andrew Morton

On Tue, 2004-11-23 at 09:56 +0100, Guillaume Thouvenin wrote:
> On Tue, 2004-11-23 at 00:06 -0800, Greg KH wrote:
> > On Tue, Nov 23, 2004 at 07:03:17AM +0100, Guillaume Thouvenin wrote:
> > > 
> > >    I don't know if this solution is good but it's easy to implement and
> > > it just does the trick. I made some tests and it doesn't impact the
> > > performance of the Linux kernel. 
> > 
> > What's wrong with the LSM hook already available for you to use for this
> > function?
> > 
> > thanks,
>
> Is it possible to get the parent PID and the child PID which are
> involved when the fork occurred?

I tried to use LSM and it works because "current" holds the pointer to
the parent. Thus, with fields "pid" and "children" I can have the
information.

So thank you very much for the hint,
Guillaume


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23  9:59 ` Hua Zhong
@ 2004-11-23 10:14   ` Guillaume Thouvenin
  0 siblings, 0 replies; 13+ messages in thread
From: Guillaume Thouvenin @ 2004-11-23 10:14 UTC (permalink / raw)
  To: hzhong; +Cc: lkml, Andrew Morton

On Tue, 2004-11-23 at 01:59 -0800, Hua Zhong wrote:
> > +	static int fork_hook_id = 0;
> > +
> > +	/* We can set the hook if it's not already used */
> > +	if ((func != NULL) && (fork_hook_id == 0)) {
> > +		fork_hook = func;
> > +		fork_hook_id = id;
> > +		return 0;
> > +	}
> 
> What happens if two modules are calling the same function at the same time?

You are right there is a problem. I need to add a lock.
 
> > +
> > +	if (fork_hook != NULL)
> > +		fork_hook(current->pid, pid);
> > +
> >  	return pid;
> 
> What happens if the module is unloaded between the test and the call to
> fork_hook?

Again you are right and I need to protect that. 

In fact, Greg suggests to use LSM hook and it seams that it does what I
need. So my patch is obsolete now. 

Thank you to everybody for your help,
Best Regards,

Guillaume


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fork: add a hook in do_fork()
  2004-11-23  6:03 [PATCH 2.6.9] fork: add a hook in do_fork() Guillaume Thouvenin
                   ` (2 preceding siblings ...)
  2004-11-23  9:59 ` Hua Zhong
@ 2004-11-23 13:55 ` Jan Engelhardt
  3 siblings, 0 replies; 13+ messages in thread
From: Jan Engelhardt @ 2004-11-23 13:55 UTC (permalink / raw)
  To: Guillaume Thouvenin; +Cc: lkml

>   I don't know if this solution is good but it's easy to implement and
>it just does the trick. I made some tests and it doesn't impact the
>performance of the Linux kernel.

Needs 2 additional CPU ops ;-)

>   I'd like to have your comment about this patch. Is it useful and is
>it needed by someone else than me?

Usually no, and I doubt there's a chance to get it in.
It's not something 60% of all kernel users, linux distros and so forth will be
going to use it.

I guess I would have the same bad luck if I was to integrate the rpldev hooks
from ttyrpld.sf.net.



Jan Engelhardt
-- 
Gesellschaft für Wissenschaftliche Datenverarbeitung
Am Fassberg, 37077 Göttingen, www.gwdg.de

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23  8:06 ` Greg KH
  2004-11-23  8:56   ` Guillaume Thouvenin
@ 2004-11-23 14:14   ` Guillaume Thouvenin
  2004-11-23 21:51     ` Chris Wright
  1 sibling, 1 reply; 13+ messages in thread
From: Guillaume Thouvenin @ 2004-11-23 14:14 UTC (permalink / raw)
  To: lkml; +Cc: guillaume.thouvenin

 So I tried to implement a solution using LSM hook and I have a strange
behavior. Here is the code of the module. It is just to test if I can
get the pid of the parent and its new created child.

--- fork_hook_module.c [BEGIN] ---

#include <linux/module.h>	/* for all modules      */
#include <linux/kernel.h>	/* for KERN_ALERT       */
#include <linux/init.h>		/* for the macros       */
#include <linux/sched.h>	/* for task_struct      */
#include <linux/security.h>     /* for LSM hook         */

static int elsa_task_alloc_security(struct task_struct *p);

struct security_operations elsa_ops = {
	.task_alloc_security = elsa_task_alloc_security,
};

static int elsa_task_alloc_security(struct task_struct *p)
{
	printk(KERN_ALERT "intercept a fork: %d created by %d\n",
	       p->pid, p->parent->pid);

	return 0;
}

static int __init fh_init(void)
{
	printk(KERN_ALERT "fh: fork hook added\n");
	if (register_security(&elsa_ops))
		panic(KERN_ALERT "fh: Unable to register fork hook\n");

	return 0;
}

static void __exit fh_exit(void)
{
	if (unregister_security(&elsa_ops))
		printk(KERN_ALERT
		       "fh: Unable to un-register with fork hook\n");

	printk(KERN_ALERT "fh: fork hook removed\n");
}

module_init(fh_init);
module_exit(fh_exit);

MODULE_AUTHOR("Guillaume Thouvenin");
MODULE_DESCRIPTION("Fork Hook");
MODULE_LICENSE("GPL");

--- fork_hook_module.c [END] ---

When I load the module, the hook is well registered. Now, if I run a
command from a shell, like a 'top', the message in the kernel log like
indicates a wrong parent ID. Here is the output of the top command:

  PID  PPID USER     %CPU CPU COMMAND
 2009  2008 guill     0.0   0 bash
 2109  2108 guill     0.0   0 bash
 2704  2109 guill     0.0   0 top

and here is the message found in the kernel log:

 intercept a fork: 2704 created by 2108

It should be 2109... not 2108
I think that the problem occurs because the security_task_alloc() is
called, the field p->parent is not set. 

Is it true? and if it is, is it possible to move the hook after the
initialization of the variable p->parent?

Regards,
Guillaume


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23  9:33   ` Guillaume Thouvenin
@ 2004-11-23 19:31     ` Jay Lan
  0 siblings, 0 replies; 13+ messages in thread
From: Jay Lan @ 2004-11-23 19:31 UTC (permalink / raw)
  To: Guillaume Thouvenin; +Cc: Christoph Hellwig, lkml, Andrew Morton, Erik Jacobson

This is great!

We have one more user of PAGG! :)

Happy Thanksgiving,
  - jay


Guillaume Thouvenin wrote:
> On Tue, 2004-11-23 at 09:03 +0000, Christoph Hellwig wrote:
> 
>>On Tue, Nov 23, 2004 at 07:03:17AM +0100, Guillaume Thouvenin wrote:
>>
>>>   For a module, I need to execute a function when a fork occurs. My
>>>solution is to add a pointer to a function (called fork_hook) in the
>>>do_fork() and if this pointer isn't NULL, I call the function. To update
>>>the pointer to the function I export a symbol (called trace_fork) that
>>>defines another function with two parameters (the hook and an
>>>identifier). This function provides a simple mechanism to manage access
>>>to the fork_hook variable.
>>>
>>
>>Use SGI's PAGG patches if you want such hooks.  Also this is clearly
>>a _GPL export.
> 
> 
> PAGG is more intrusive than my patch due to the management of groups of
> processes. This hook in the fork allows me to provide a solution to do
> per-group accounting with a module. If PAGG is added in the Linux Kernel
> Tree it could be the solution, you are right. 
> 
> Guillaume 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23 14:14   ` Guillaume Thouvenin
@ 2004-11-23 21:51     ` Chris Wright
  2004-11-24  8:14       ` Guillaume Thouvenin
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Wright @ 2004-11-23 21:51 UTC (permalink / raw)
  To: Guillaume Thouvenin; +Cc: lkml

* Guillaume Thouvenin (Guillaume.Thouvenin@Bull.net) wrote:
> static int elsa_task_alloc_security(struct task_struct *p)
> {
> 	printk(KERN_ALERT "intercept a fork: %d created by %d\n",
> 	       p->pid, p->parent->pid);
		       
It's created by current.  So, current->pid.  p is not completely setup
yet, and is still largely duplication of current from dup_task_struct().

>   PID  PPID USER     %CPU CPU COMMAND
>  2009  2008 guill     0.0   0 bash
>  2109  2108 guill     0.0   0 bash
>  2704  2109 guill     0.0   0 top
> 
> and here is the message found in the kernel log:
> 
>  intercept a fork: 2704 created by 2108
> 
> It should be 2109... not 2108
> I think that the problem occurs because the security_task_alloc() is
> called, the field p->parent is not set. 
> 
> Is it true? and if it is, is it possible to move the hook after the
> initialization of the variable p->parent?

No, it's correct where it is.  And, IIRC, elsa is accounting related.
LSM is not the right framework, you should be using something like PAGG
or CKRM.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2.6.9] fork: add a hook in do_fork()
  2004-11-23 21:51     ` Chris Wright
@ 2004-11-24  8:14       ` Guillaume Thouvenin
  0 siblings, 0 replies; 13+ messages in thread
From: Guillaume Thouvenin @ 2004-11-24  8:14 UTC (permalink / raw)
  To: Chris Wright; +Cc: lkml

On Tue, 2004-11-23 at 13:51 -0800, Chris Wright wrote:
> * Guillaume Thouvenin (Guillaume.Thouvenin@Bull.net) wrote:
> > static int elsa_task_alloc_security(struct task_struct *p)
> > {
> > 	printk(KERN_ALERT "intercept a fork: %d created by %d\n",
> > 	       p->pid, p->parent->pid);
> 		       
> It's created by current.  So, current->pid.  p is not completely setup
> yet, and is still largely duplication of current from dup_task_struct().

I see. Thus the correct answer is: process pointed by "current" is the
parent of the process pointed by "p" when elsa_task_alloc_security() is
called. 

> And, IIRC, elsa is accounting related.
> LSM is not the right framework, you should be using something like PAGG
> or CKRM.

I understand your point of view. Elsa is accounting related that's true
but I'm trying to provide a solution without modifying the Linux kernel
tree. To achieve this I just need a hook in the fork to be inform when a
process creates a child. LSM hook does the trick and it is already in
the kernel. That's why I use the LSM hook (and I'm waiting to see PAGG
or CKRM in the Linux kernel).

Thanks,
Guillaume


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2004-11-24  8:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-11-23  6:03 [PATCH 2.6.9] fork: add a hook in do_fork() Guillaume Thouvenin
2004-11-23  8:06 ` Greg KH
2004-11-23  8:56   ` Guillaume Thouvenin
2004-11-23 10:09     ` Guillaume Thouvenin
2004-11-23 14:14   ` Guillaume Thouvenin
2004-11-23 21:51     ` Chris Wright
2004-11-24  8:14       ` Guillaume Thouvenin
2004-11-23  9:03 ` Christoph Hellwig
2004-11-23  9:33   ` Guillaume Thouvenin
2004-11-23 19:31     ` Jay Lan
2004-11-23  9:59 ` Hua Zhong
2004-11-23 10:14   ` Guillaume Thouvenin
2004-11-23 13:55 ` Jan Engelhardt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox