Re: [PATCH resend] audit: fix mark refcounting

[Eric Paris]

On Thu, 2011-12-15 at 21:06 +0100, Lino Sanfilippo wrote:
> On Thu, Dec 15, 2011 at 10:03:41AM +0100, Miklos Szeredi wrote:
> >  
> > +	audit_get_parent(parent);
> >  	fsnotify_destroy_mark(&parent->mark);
> > +	audit_put_parent(parent);
> 
> Hi,
> 
> What about taking an extra ref on an inode mark in send_to_group()
> before we call handle_event()?
> So we dont have to handle the cases in which a mark is destroyed
> explicitly...

Yes, but it puts atomic operations on a much hotter path.  I don't
believe there is actually a bug in the code here, but clearly we're
tripping over a BUG() I put in the code.  These patches shut up the
BUG() with minimal overhead, but don't really solve any problem.

Lino is right, the 'right' place to do this would be to take a reference
in fs/notify/fsnotify.c::fsnotify() when we find a mark under the srcu
lock and drop that reference when we are finished using that mark.
Since that function only ever uses the mark to call the handler() if it
gets destroyed in the handler that is fine.  We'd have a real bug here
if fsnotify() used the mark after the handler call...

How expensive is an atomic_inc()/atomic_dec() combo?  If it's mostly
free we can do it in the right place.

-Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/