2004-11-23 06:07:05

by Guillaume Thouvenin

[permalink] [raw]
Subject: [PATCH 2.6.9] fork: add a hook in do_fork()

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 <[email protected]>

--- 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;
}



2004-11-23 08:07:27

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 2.6.9] fork: add a hook in do_fork()

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

2004-11-23 08:56:49

by Guillaume Thouvenin

[permalink] [raw]
Subject: Re: [PATCH 2.6.9] fork: add a hook in do_fork()

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

2004-11-23 09:03:34

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2.6.9] fork: add a hook in do_fork()

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.

2004-11-23 09:33:35

by Guillaume Thouvenin

[permalink] [raw]
Subject: Re: [PATCH 2.6.9] fork: add a hook in do_fork()

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


2004-11-23 09:59:38

by Hua Zhong (hzhong)

[permalink] [raw]
Subject: RE: [PATCH 2.6.9] fork: add a hook in do_fork()

> + 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

2004-11-23 10:10:26

by Guillaume Thouvenin

[permalink] [raw]
Subject: Re: [PATCH 2.6.9] fork: add a hook in do_fork()

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

2004-11-23 10:16:36

by Guillaume Thouvenin

[permalink] [raw]
Subject: RE: [PATCH 2.6.9] fork: add a hook in do_fork()

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

2004-11-23 13:56:52

by Jan Engelhardt

[permalink] [raw]
Subject: Re: fork: add a hook in do_fork()

> 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, http://www.gwdg.de

2004-11-23 14:15:50

by Guillaume Thouvenin

[permalink] [raw]
Subject: Re: [PATCH 2.6.9] fork: add a hook in do_fork()

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

2004-11-23 19:34:54

by Jay Lan

[permalink] [raw]
Subject: Re: [PATCH 2.6.9] fork: add a hook in do_fork()

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
>

2004-11-23 21:54:54

by Chris Wright

[permalink] [raw]
Subject: Re: [PATCH 2.6.9] fork: add a hook in do_fork()

* Guillaume Thouvenin ([email protected]) 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

2004-11-24 08:17:58

by Guillaume Thouvenin

[permalink] [raw]
Subject: Re: [PATCH 2.6.9] fork: add a hook in do_fork()

On Tue, 2004-11-23 at 13:51 -0800, Chris Wright wrote:
> * Guillaume Thouvenin ([email protected]) 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