linux-kernel - Re: [PATCH resend] audit: fix mark refcounting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 15 Dec 2011 17:28:12 -0500
From:	Eric Paris <eparis@...hat.com>
To:	Lino Sanfilippo <LinoSanfilippo@....de>
Cc:	Miklos Szeredi <miklos@...redi.hu>,
	Al Viro <viro@...IV.linux.org.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	akpm@...ux-foundation.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH resend] audit: fix mark refcounting

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/