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@xxxxxxxx>
SUSE Labs, CR
