| Message ID | 1473797711-14111-6-git-send-email-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13.09.2016 22:15, Jan Kara wrote: > Fanotify code has an own lock (access_lock) to protect a list of events > waiting for a response from userspace. However this is somewhat awkward > as the same list_head in the event is protected by notification_lock > if it is part of the notification queue and by access_lock if it is part > of the fanotify private queue which makes it difficult for any reliable > checks in the generic code. So make fanotify use the same lock - > notification_lock - for protecting its private event list. > > Signed-off-by: Jan Kara <jack@suse.cz> > --- > fs/notify/fanotify/fanotify_user.c | 13 +++++-------- > include/linux/fsnotify_backend.h | 1 - > 2 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index e6f3fe9bb2ed..aed06fe2533d 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -147,7 +147,7 @@ static struct fanotify_perm_event_info *dequeue_event( > { > struct fanotify_perm_event_info *event, *return_e = NULL; > > - spin_lock(&group->fanotify_data.access_lock); > + spin_lock(&group->notification_lock); > list_for_each_entry(event, &group->fanotify_data.access_list, > fae.fse.list) { > if (event->fd != fd) > @@ -157,7 +157,7 @@ static struct fanotify_perm_event_info *dequeue_event( > return_e = event; > break; > } > - spin_unlock(&group->fanotify_data.access_lock); > + spin_unlock(&group->notification_lock); > > pr_debug("%s: found return_re=%p\n", __func__, return_e); > > @@ -309,10 +309,10 @@ static ssize_t fanotify_read(struct file *file, char __user *buf, > wake_up(&group->fanotify_data.access_waitq); > break; > } > - spin_lock(&group->fanotify_data.access_lock); > + spin_lock(&group->notification_lock); > list_add_tail(&kevent->list, > &group->fanotify_data.access_list); > - spin_unlock(&group->fanotify_data.access_lock); > + spin_unlock(&group->notification_lock); > #endif > } > buf += ret; > @@ -371,7 +371,7 @@ static int fanotify_release(struct inode *ignored, struct file *file) > * Process all permission events on access_list and notification queue > * and simulate reply from userspace. > */ > - spin_lock(&group->fanotify_data.access_lock); > + spin_lock(&group->notification_lock); > list_for_each_entry_safe(event, next, &group->fanotify_data.access_list, > fae.fse.list) { > pr_debug("%s: found group=%p event=%p\n", __func__, group, > @@ -380,14 +380,12 @@ static int fanotify_release(struct inode *ignored, struct file *file) > list_del_init(&event->fae.fse.list); > event->response = FAN_ALLOW; > } > - spin_unlock(&group->fanotify_data.access_lock); > > /* > * Destroy all non-permission events. For permission events just > * dequeue them and set the response. They will be freed once the > * response is consumed and fanotify_get_response() returns. > */ > - spin_lock(&group->notification_lock); > while (!fsnotify_notify_queue_is_empty(group)) { > fsn_event = fsnotify_remove_first_event(group); > if (!(fsn_event->mask & FAN_ALL_PERM_EVENTS)) { > @@ -767,7 +765,6 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags) > event_f_flags |= O_LARGEFILE; > group->fanotify_data.f_flags = event_f_flags; > #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS > - spin_lock_init(&group->fanotify_data.access_lock); > init_waitqueue_head(&group->fanotify_data.access_waitq); > INIT_LIST_HEAD(&group->fanotify_data.access_list); > #endif > diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h > index 0713e873b1c9..79467b239fcf 100644 > --- a/include/linux/fsnotify_backend.h > +++ b/include/linux/fsnotify_backend.h > @@ -177,7 +177,6 @@ struct fsnotify_group { > struct fanotify_group_private_data { > #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS > /* allows a group to block waiting for a userspace response */ > - spinlock_t access_lock; > struct list_head access_list; > wait_queue_head_t access_waitq; > #endif /* CONFIG_FANOTIFY_ACCESS_PERMISSIONS */ > 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 e6f3fe9bb2ed..aed06fe2533d 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -147,7 +147,7 @@ static struct fanotify_perm_event_info *dequeue_event( { struct fanotify_perm_event_info *event, *return_e = NULL; - spin_lock(&group->fanotify_data.access_lock); + spin_lock(&group->notification_lock); list_for_each_entry(event, &group->fanotify_data.access_list, fae.fse.list) { if (event->fd != fd) @@ -157,7 +157,7 @@ static struct fanotify_perm_event_info *dequeue_event( return_e = event; break; } - spin_unlock(&group->fanotify_data.access_lock); + spin_unlock(&group->notification_lock); pr_debug("%s: found return_re=%p\n", __func__, return_e); @@ -309,10 +309,10 @@ static ssize_t fanotify_read(struct file *file, char __user *buf, wake_up(&group->fanotify_data.access_waitq); break; } - spin_lock(&group->fanotify_data.access_lock); + spin_lock(&group->notification_lock); list_add_tail(&kevent->list, &group->fanotify_data.access_list); - spin_unlock(&group->fanotify_data.access_lock); + spin_unlock(&group->notification_lock); #endif } buf += ret; @@ -371,7 +371,7 @@ static int fanotify_release(struct inode *ignored, struct file *file) * Process all permission events on access_list and notification queue * and simulate reply from userspace. */ - spin_lock(&group->fanotify_data.access_lock); + spin_lock(&group->notification_lock); list_for_each_entry_safe(event, next, &group->fanotify_data.access_list, fae.fse.list) { pr_debug("%s: found group=%p event=%p\n", __func__, group, @@ -380,14 +380,12 @@ static int fanotify_release(struct inode *ignored, struct file *file) list_del_init(&event->fae.fse.list); event->response = FAN_ALLOW; } - spin_unlock(&group->fanotify_data.access_lock); /* * Destroy all non-permission events. For permission events just * dequeue them and set the response. They will be freed once the * response is consumed and fanotify_get_response() returns. */ - spin_lock(&group->notification_lock); while (!fsnotify_notify_queue_is_empty(group)) { fsn_event = fsnotify_remove_first_event(group); if (!(fsn_event->mask & FAN_ALL_PERM_EVENTS)) { @@ -767,7 +765,6 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags) event_f_flags |= O_LARGEFILE; group->fanotify_data.f_flags = event_f_flags; #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS - spin_lock_init(&group->fanotify_data.access_lock); init_waitqueue_head(&group->fanotify_data.access_waitq); INIT_LIST_HEAD(&group->fanotify_data.access_list); #endif diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index 0713e873b1c9..79467b239fcf 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -177,7 +177,6 @@ struct fsnotify_group { struct fanotify_group_private_data { #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS /* allows a group to block waiting for a userspace response */ - spinlock_t access_lock; struct list_head access_list; wait_queue_head_t access_waitq; #endif /* CONFIG_FANOTIFY_ACCESS_PERMISSIONS */
Fanotify code has an own lock (access_lock) to protect a list of events waiting for a response from userspace. However this is somewhat awkward as the same list_head in the event is protected by notification_lock if it is part of the notification queue and by access_lock if it is part of the fanotify private queue which makes it difficult for any reliable checks in the generic code. So make fanotify use the same lock - notification_lock - for protecting its private event list. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/notify/fanotify/fanotify_user.c | 13 +++++-------- include/linux/fsnotify_backend.h | 1 - 2 files changed, 5 insertions(+), 9 deletions(-)