From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:47158 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752783AbdDCPHk (ORCPT ); Mon, 3 Apr 2017 11:07:40 -0400 Date: Mon, 3 Apr 2017 17:07:37 +0200 From: Jan Kara To: Miklos Szeredi Cc: Jan Kara , linux-fsdevel@vger.kernel.org, Amir Goldstein , Paul Moore Subject: Re: [PATCH 20/33] fsnotify: Detach mark from object list when last reference is dropped Message-ID: <20170403150737.GG15168@quack2.suse.cz> References: <20170328161332.29736-1-jack@suse.cz> <20170328161332.29736-21-jack@suse.cz> <20170331154217.GC25833@veci.piliscsaba.szeredi.hu> <20170403100406.GC15168@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170403100406.GC15168@quack2.suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 SUSE Labs, CR