On Wednesday 11 May 2005 14:51, serue(a)us.ibm.com wrote:
 Quoting Timothy R. Chavez (tinytim(a)us.ibm.com):
 > On Wednesday 11 May 2005 14:01, serue(a)us.ibm.com wrote:
 > > Quoting Timothy R. Chavez (tinytim(a)us.ibm.com):
 > > > * I've completely removed the audit_master_watchlist_lock spinlock to
 > > > protect the master watchlist, in favor of RCU locking. ?To protect
 > > > against concurrent watch removals from the master watchlist, we
 > > > conveniently use the local data->lock rw_lock in place of another
 > > > spinlock. ?Thus, only one removal can
 > >
 > > I have a problem with this.  Isn't the audit_master_watchlist a global
 > > list?  Are you sure you can use a per-inode lock to protect this global
 > > list?
 >
 > Well, my conclusion was this:  The only way to enter
 > audit_destroy_wentry() where contention is a concern is by holding the
 > local data->lock.  The only way we can remove a watch from the master
 > watchlist is by entering the audit_destroy_wentry() function (and if
 > contentious, only one of the contenders may be in audit_destroy_wentry()
 > at a time).
 >
 > I don't see how we could race on a master watchlist deletion.
 1. What if you end up trying to delete two wentries which are adjecent
 on the audit_master_watchlist concurrently?  I think you might be able
 to break the list, but I haven't drawn it down on paper... 
Hm.  Yeah, sounds like in this case, it'll break.  I'll have to think about 
this scenario a bit more.
 2. What about additions to the audit_master_watchlist, both racing with
 each other and with deletions?  Actually it doesn't look like you're
 protecting additions at all.
 
There won't be racing insertions.  The audit_netlink_sem ensures us that there 
will only ever be one insertion at a time and while we're inserting.  Also, 
if we are inserting, we can't explicitly be removing.  The only way to delete 
is from audit_update_watch().  So conceptually, I suppose, if we're in the 
process of draining the watchlist and we're attempting to insert into that 
watchlist at the same time, there might be some ill effect?  I suppose it 
doesn't hurt to keep the locking here for this case.
 -serge
 --
 Linux-audit mailing list
 Linux-audit(a)redhat.com
 
http://www.redhat.com/mailman/listinfo/linux-audit 
-- 
-tim