From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net ([212.227.15.15]:54810 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752028AbcINRLp (ORCPT ); Wed, 14 Sep 2016 13:11:45 -0400 Subject: Re: [PATCH 3/6] fsnotify: Drop notification_mutex before destroying event To: Jan Kara , Andrew Morton References: <1473797711-14111-1-git-send-email-jack@suse.cz> <1473797711-14111-4-git-send-email-jack@suse.cz> Cc: linux-fsdevel@vger.kernel.org, Miklos Szeredi , Eric Paris , Al Viro From: Lino Sanfilippo Message-ID: Date: Wed, 14 Sep 2016 19:11:34 +0200 MIME-Version: 1.0 In-Reply-To: <1473797711-14111-4-git-send-email-jack@suse.cz> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 13.09.2016 22:15, Jan Kara wrote: > fsnotify_flush_notify() and fanotify_release() destroy notification > event while holding notification_mutex. Destruction of fanotify event > includes a path_put() call which may end up calling into a filesystem to > delete an inode if we happen to be the last holders of dentry reference > which happens to be the last holder of inode reference. That may violate > lock ordering for some filesystems since notification_mutex is also > acquired e. g. during write when generating fanotify event. Also this is > the only thing that forces notification_mutex to be a sleeping lock. So > drop notification_mutex before destroying a notification event. > > Signed-off-by: Jan Kara > --- > fs/notify/fanotify/fanotify_user.c | 6 ++++-- > fs/notify/notification.c | 2 ++ > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index a64313868d3a..46d135c4988f 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -390,9 +390,11 @@ static int fanotify_release(struct inode *ignored, struct file *file) > mutex_lock(&group->notification_mutex); > while (!fsnotify_notify_queue_is_empty(group)) { > fsn_event = fsnotify_remove_first_event(group); > - if (!(fsn_event->mask & FAN_ALL_PERM_EVENTS)) > + if (!(fsn_event->mask & FAN_ALL_PERM_EVENTS)) { > + mutex_unlock(&group->notification_mutex); > fsnotify_destroy_event(group, fsn_event); > - else > + mutex_lock(&group->notification_mutex); > + } else > FANOTIFY_PE(fsn_event)->response = FAN_ALLOW; > } > mutex_unlock(&group->notification_mutex); > diff --git a/fs/notify/notification.c b/fs/notify/notification.c > index e455e83ceeeb..7d563dea52a4 100644 > --- a/fs/notify/notification.c > +++ b/fs/notify/notification.c > @@ -178,7 +178,9 @@ void fsnotify_flush_notify(struct fsnotify_group *group) > mutex_lock(&group->notification_mutex); > while (!fsnotify_notify_queue_is_empty(group)) { > event = fsnotify_remove_first_event(group); > + mutex_unlock(&group->notification_mutex); > fsnotify_destroy_event(group, event); > + mutex_lock(&group->notification_mutex); > } > mutex_unlock(&group->notification_mutex); > } > Reviewed-by: Lino Sanfilippo