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

Re: [RFC][PATCH] (#6 U1) the latest incarnation



On Friday 25 March 2005 07:28 am, Stephen Smalley wrote:
> On Fri, 2005-03-25 at 08:04 -0500, Stephen Smalley wrote:
> > With regard to additional hook placement for audit_notify_watch, I think
> > you likely do want to mirror the security*_post* hooks for file creation
> > (create, mkdir, mknod, symlink), rename, and link with
> > audit_notify_watch calls to perform notifications of such events.  Then
> > you keep audit_attach_watch calls in the dcache routines to manage the
> > i_audit fields and avoid races.  However, I think you need to check
> > whether you truly need all of the current hook placements in the dcache
> > routines or whether some of them are duplicative on the same code path,
> > e.g. do you need both __d_lookup and d_instantiate/d_splice_alias
> > hooked?
>
> I don't see what the __d_lookup hook buys you, can you explain?  The
> d_instantiate and d_splice_alias hooks ensure that you attach watches to
> inodes when they are looked up or created before they can be accessed by
> another thread via the dcache (since you call the hook before releasing
> the dcache lock).  What do you need the __d_lookup hook for?

Hello,

I've kind of struggled with this one and am was a bit reluctant to add it.  
Perhaps my logic is right, bu there's a better placement.  The reason why the 
hook was placed in __d_lookup() was to auto-update a hardlink with the 
correct watch.  The only way a hardlink will generate audit records is if 
it's inode is being watched and the only way the inode can be watched is if 
one of it's dentry's is at a watch point.  So, take this scenario for example 
-- this is how we should currently perform:

watch /tmp/foo
# We get a record for "foo"
cat /tmp/foo
ln /tmp/foo /tmp/bar
watch /tmp/bar
# We get a record for "foo"
cat /tmp/bar
rm /tmp/foo
# __d_lookup() does its magic and we get a record for "bar"
cat /tmp/bar

This might be a completely assanine case now....

If /tmp/bar has a "natural" watch (ie: there's a watch on /tmp for "bar"), 
and /tmp/bar is a hardlink to /tmp/foo, then if /tmp/foo left (it was the 
dentry for the inode that was fufilling the watch criteria), then the inode 
would lose its "foo" watch and /tmp/bar would be watchless.  But /tmp/bar is 
being watched!  So we use __d_lookup() to automatically detect whether or not 
there is a "natural" watch in its current path that it can attach too.  This 
could get a little more interesting if there are N hardlinks to /tmp/foo 
which are all being watched.  Then, any of the /tmp/XN's could claim the 
inode and attach their watch -- it depends on what /tmp/XN calls the 
audiit_attach_watch() first.

Comments? Opinions?

-tim


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