| Message ID | 1473797711-14111-4-git-send-email-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <jack@suse.cz> > --- > 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 <LinoSanfilippo@gmx.de> -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); }
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 <jack@suse.cz> --- fs/notify/fanotify/fanotify_user.c | 6 ++++-- fs/notify/notification.c | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-)