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:
http://www.ussg.iu.edu/hypermail/linux/kernel/0606.1/1800.html
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:
copy_process()
{
...
notify_watchers(WATCH_TASK_INIT, p);
...
if (<succeeding>)
notify_watchers(WATCH_TASK_CLONE, p);
...
bad_foo:
...
- 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.>
do_exit()
{
...
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)) {
case WATCH_TASK_INIT:
+ tsk = v; /* INIT and ABORT use v, the rest use current */
...
+ case WATCH_TASK_ABORT:
+ tsk = v; /* fall through */
case WATCH_TASK_FREE:
...
}
...
}
Which seems a bit more complicated. Is this worth it?
Cheers,
-Matt Helsley
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
