From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:50451 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753181AbdDEHiu (ORCPT ); Wed, 5 Apr 2017 03:38:50 -0400 Date: Wed, 5 Apr 2017 09:38:48 +0200 From: Jan Kara To: Paul Moore Cc: Jan Kara , linux-fsdevel@vger.kernel.org, Miklos Szeredi , Amir Goldstein Subject: Re: [PATCH 05/35] audit_tree: Use mark flags to check whether mark is alive Message-ID: <20170405073848.GB8899@quack2.suse.cz> References: <20170403153424.24945-1-jack@suse.cz> <20170403153424.24945-6-jack@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue 04-04-17 16:47:11, Paul Moore wrote: > On Mon, Apr 3, 2017 at 11:33 AM, Jan Kara wrote: > > Currently audit code uses checking of mark->inode to verify whether mark > > is still alive. Switch that to checking mark flags as that is more > > logical and current way will become unreliable in future. > > > > Reviewed-by: Miklos Szeredi > > Signed-off-by: Jan Kara > > --- > > kernel/audit_tree.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Should audit_tree.c:insert_hash() also be updated in a similar manner? Do you mean the part which has become chunk_to_key()? The code there is correct as is since we just use the value of inode pointer for hashing but never dereference it. Also some of the callers of chunk_to_key() don't hold locks that would prevent chunk.mark->connector->inode becoming NULL just after we checked the FSNOTIFY_MARK_FLAG_ATTACHED flag so in practice nothing would change. So I've decided to leave that code as is. That being said I don't have a strong opinion about it so if you prefer checking the flag, I can do it that way. Honza > > > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c > > index 11c7ac441624..f12bd40fb8f1 100644 > > --- a/kernel/audit_tree.c > > +++ b/kernel/audit_tree.c > > @@ -248,7 +248,7 @@ static void untag_chunk(struct node *p) > > > > mutex_lock(&entry->group->mark_mutex); > > spin_lock(&entry->lock); > > - if (chunk->dead || !entry->inode) { > > + if (chunk->dead || !(entry->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) { > > spin_unlock(&entry->lock); > > mutex_unlock(&entry->group->mark_mutex); > > if (new) > > @@ -408,7 +408,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree) > > > > mutex_lock(&old_entry->group->mark_mutex); > > spin_lock(&old_entry->lock); > > - if (!old_entry->inode) { > > + if (!(old_entry->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) { > > /* old_entry is being shot, lets just lie */ > > spin_unlock(&old_entry->lock); > > mutex_unlock(&old_entry->group->mark_mutex); > > -- > > 2.10.2 > > > > -- > paul moore > www.paul-moore.com -- Jan Kara SUSE Labs, CR