On Wed, Feb 21, 2018 at 5:44 PM, Jan Kara <jack@xxxxxxx> wrote:
> Currently if notification event is lost due to event allocation failing
> we ENOMEM, we just silently continue (except for fanotify permission
> events where we deny the access). This is undesirable as userspace has
> no way of knowing whether the notifications it got are complete or not.
> Treat lost events due to ENOMEM the same way as lost events due to queue
> overflow so that userspace knows something bad happened and it likely
> needs to rescan the filesystem.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/notify/fanotify/fanotify.c | 9 ++++++++-
> fs/notify/inotify/inotify_fsnotify.c | 8 +++++++-
> fs/notify/notification.c | 3 ++-
> include/linux/fsnotify_backend.h | 6 ++++++
> 4 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 928f2a5eedb7..d51e1bb781cf 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -221,8 +221,15 @@ static int fanotify_handle_event(struct fsnotify_group *group,
>
> event = fanotify_alloc_event(group, inode, mask, data);
> ret = -ENOMEM;
> - if (unlikely(!event))
> + if (unlikely(!event)) {
> + /*
> + * We don't queue overflow events for permission events as
> + * there the access is denied and so no event is in fact lost.
> + */
> + if (!fanotify_is_perm_event(mask))
> + fsnotify_queue_overflow(group);
> goto finish;
> + }
>
> fsn_event = &event->fse;
> ret = fsnotify_add_event(group, fsn_event, fanotify_merge);
> diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
> index 8b73332735ba..6440df8fb29a 100644
> --- a/fs/notify/inotify/inotify_fsnotify.c
> +++ b/fs/notify/inotify/inotify_fsnotify.c
> @@ -99,8 +99,14 @@ int inotify_handle_event(struct fsnotify_group *group,
> fsn_mark);
>
> event = kmalloc(alloc_len, GFP_KERNEL);
> - if (unlikely(!event))
> + if (unlikely(!event)) {
> + /*
> + * Treat lost event due to ENOMEM the same way as queue
> + * overflow to let userspace know event was lost.
> + */
> + fsnotify_queue_overflow(group);
> return -ENOMEM;
> + }
>
> fsn_event = &event->fse;
> fsnotify_init_event(fsn_event, inode, mask);
> diff --git a/fs/notify/notification.c b/fs/notify/notification.c
> index 66f85c651c52..3c3e36745f59 100644
> --- a/fs/notify/notification.c
> +++ b/fs/notify/notification.c
> @@ -111,7 +111,8 @@ int fsnotify_add_event(struct fsnotify_group *group,
> return 2;
> }
>
> - if (group->q_len >= group->max_events) {
> + if (event == group->overflow_event ||
> + group->q_len >= group->max_events) {
> ret = 2;
> /* Queue overflow event only if it isn't already queued */
> if (!list_empty(&group->overflow_event->list)) {
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index 067d52e95f02..9f1edb92c97e 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -331,6 +331,12 @@ extern int fsnotify_add_event(struct fsnotify_group *group,
> struct fsnotify_event *event,
> int (*merge)(struct list_head *,
> struct fsnotify_event *));
> +/* Queue overflow event to a notification group */
> +static inline void fsnotify_queue_overflow(struct fsnotify_group *group)
> +{
> + fsnotify_add_event(group, group->overflow_event, NULL);
> +}
> +
> /* true if the group notification queue is empty */
> extern bool fsnotify_notify_queue_is_empty(struct fsnotify_group *group);
> /* return, but do not dequeue the first event on the notification queue */
> --
> 2.13.6
>
