Re: [PATCH 20/33] fsnotify: Detach mark from object list when last reference is dropped

[Jan Kara <jack@suse.cz>]

On Mon 03-04-17 12:04:06, Jan Kara wrote:
> On Fri 31-03-17 17:42:17, Miklos Szeredi wrote:
> > > @@ -202,7 +190,7 @@ static inline struct list_head *chunk_hash(unsigned long key)
> > >  /* hash_lock & entry->lock is held by caller */
> > >  static void insert_hash(struct audit_chunk *chunk)
> > >  {
> > > -	unsigned long key = __chunk_to_key(chunk);
> > > +	unsigned long key = chunk_to_key(chunk);
> > >  	struct list_head *list;
> > >  
> > >  	if (!key)
> > > @@ -263,7 +251,7 @@ static void untag_chunk(struct node *p)
> > >  
> > >  	mutex_lock(&entry->group->mark_mutex);
> > >  	spin_lock(&entry->lock);
> > > -	if (chunk->dead || !entry->connector) {
> > > +	if (chunk->dead || !entry->connector || !entry->connector->inode) {
> > 
> > So should we be testing FSNOTIFY_MARK_FLAG_ATTACHED instead?  Without
> > understanding what audit is trying to do, that would be a lot more
> > logical.  Or maybe there is a reason this is correct, it just needs an
> > explanation.
> 
> That's an interesting idea. AFAIU checking FSNOTIFY_MARK_FLAG_ATTACHED
> should be good and doing so would somewhat simplify the patches as well.
> I'll try to do that.

OK, I did this and also realized that this actually fixes a possible NULL
pointer dereference bug introduced by my series (entry->connector->inode
can become NULL if 'entry' is already detached but inode is still valid at
the time of the check). So I also added a comment about this.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR