[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH 06/11] Task watchers: Register audit task watcher

On Wed, 2006-06-14 at 10:46 -0400, Alexander Viro wrote:
> On Tue, Jun 13, 2006 at 04:54:46PM -0700, Matt Helsley wrote:
> > Adapt audit to use task watchers.
> audit_free(p) really expects that either p is a stillborn (never ran)
> *or* that p == current.

	Makes sense. I think the task watcher patches are consistent with this.
I think the first patch of this series helps explain how this patch
remains consistent with the above. I should have cc'd linux-audit when
posting that patch -- here's a link for now:

	In copy_process() and do_exit() notify_watchers() passes the same
pointers as audit_alloc() and audit_free() used before. The patches also
do not introduce or remove calls to audit_alloc() or audit_free(). The
patches trigger these calls with notify_watchers() while passing
WATCH_TASK_INIT and WATCH_TASK_FREE for audit_alloc() and audit_free()
respectively. WATCH_TASK_INIT (and hence audit_alloc()) only happens in
copy_process(). WATCH_TASK_FREE (and hence audit_free()) happens in
copy_process()'s error recovery path and in do_exit().

	This results in the same calls to audit_alloc() and audit_free() except
with an additional function call preceding them on the stack.

	Are you concerned that future modifications of task watchers would pass
in task structs that violate these expectations? I can alter the patches
to incorporate these restrictions:

	notify_watchers(WATCH_TASK_INIT, p);
	if (<succeeding>)
		notify_watchers(WATCH_TASK_CLONE, p);
- 	notify_watchers(WATCH_TASK_FREE, p);
+ 	notify_watchers(WATCH_TASK_ABORT, p);

<change all other notify_watchers() invocations to pass NULL as
the second parameter, e.g.>

	notify_watchers(WATCH_TSK_FREE, NULL); /* callees must use current */

However this requires that I modify each user of task watchers with
something like:

int foo (struct notifier_block *nb, unsigned long val, void *v)
-	struct task_struct *tsk = v;
+	struct task_struct *tsk = current;
	switch(get_watch_event(val)) {
+ 		tsk = v; /* INIT and ABORT use v, the rest use current */
+ 		tsk = v; /* fall through */

Which seems a bit more complicated. Is this worth it?

	-Matt Helsley

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]