diff mbox

[05/35] audit_tree: Use mark flags to check whether mark is alive

Message ID 20170410154340.GJ3224@quack2.suse.cz (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kara April 10, 2017, 3:43 p.m. UTC
On Mon 10-04-17 17:31:26, Jan Kara wrote:
> On Thu 06-04-17 07:51:48, Paul Moore wrote:
> > On Wed, Apr 5, 2017 at 3:38 AM, Jan Kara <jack@suse.cz> wrote:
> > > On Tue 04-04-17 16:47:11, Paul Moore wrote:
> > >> On Mon, Apr 3, 2017 at 11:33 AM, Jan Kara <jack@suse.cz> 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 <mszeredi@redhat.com>
> > >> > Signed-off-by: Jan Kara <jack@suse.cz>
> > >> > ---
> > >> >  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()? ...
> > 
> > No, I was talking about the if conditional near the top of the
> > function that checks to see if the fsnotify_mark's inode is non-NULL;
> > it seems like you would also want to convert that to a
> > FSNOTIFY_MARK_FLAG_ATTACHED, yes?
> 
> Ah, that one. Yes, that can certainly become a FSNOTIFY_MARK_FLAG_ATTACHED
> check although I cannot currently come up with a situation where it would
> matter. But it looks safer that way so I'll change that check as you
> suggest.

Attached is the resulting patch.

								Honza

Comments

Paul Moore April 11, 2017, 8:27 p.m. UTC | #1
On Mon, Apr 10, 2017 at 11:43 AM, Jan Kara <jack@suse.cz> wrote:
> On Mon 10-04-17 17:31:26, Jan Kara wrote:
>> On Thu 06-04-17 07:51:48, Paul Moore wrote:
>> > On Wed, Apr 5, 2017 at 3:38 AM, Jan Kara <jack@suse.cz> wrote:
>> > > On Tue 04-04-17 16:47:11, Paul Moore wrote:
>> > >> On Mon, Apr 3, 2017 at 11:33 AM, Jan Kara <jack@suse.cz> 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 <mszeredi@redhat.com>
>> > >> > Signed-off-by: Jan Kara <jack@suse.cz>
>> > >> > ---
>> > >> >  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()? ...
>> >
>> > No, I was talking about the if conditional near the top of the
>> > function that checks to see if the fsnotify_mark's inode is non-NULL;
>> > it seems like you would also want to convert that to a
>> > FSNOTIFY_MARK_FLAG_ATTACHED, yes?
>>
>> Ah, that one. Yes, that can certainly become a FSNOTIFY_MARK_FLAG_ATTACHED
>> check although I cannot currently come up with a situation where it would
>> matter. But it looks safer that way so I'll change that check as you
>> suggest.
>
> Attached is the resulting patch.

<grumpy old maintainer voice>
Please post patches inline, it is so much easier for me to review that way.
</grumpy old maintainer voice>

Acked-by: Paul Moore <paul@paul-moore.com>
diff mbox

Patch

From 43471d15df0e7c40ca4df1513fc1dcf5765396ac Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Mon, 3 Apr 2017 16:47:58 +0200
Subject: [PATCH] audit_tree: Use mark flags to check whether mark is alive

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 <mszeredi@redhat.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 kernel/audit_tree.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 11c7ac441624..51451245936a 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -190,7 +190,7 @@  static void insert_hash(struct audit_chunk *chunk)
 	unsigned long key = chunk_to_key(chunk);
 	struct list_head *list;
 
-	if (!key)
+	if (!(chunk->mark.flags & FSNOTIFY_MARK_FLAG_ATTACHED))
 		return;
 	list = chunk_hash(key);
 	list_add_rcu(&chunk->hash, list);
@@ -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.12.0