* [PATCH 0/2] fsnotify: Improve ENOMEM handling @ 2018-02-21 15:44 Jan Kara 2018-02-21 15:44 ` [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues Jan Kara 2018-02-21 15:44 ` [PATCH 2/2] fsnotify: Let userspace know about lost events due to ENOMEM Jan Kara 0 siblings, 2 replies; 9+ messages in thread From: Jan Kara @ 2018-02-21 15:44 UTC (permalink / raw) To: linux-fsdevel; +Cc: Shakeel Butt, Amir Goldstein, Jan Kara Hello, these two patches improve ENOMEM handling in fsnotify framework. In particular we queue overflow event if we cannot report some event due to ENOMEM (currently we just silently lose the event). We also make sure fanotify unlimited queues cannot lose events due to ENOMEM. This series is prerequisite for memcg charging of fsnotify events. Comments are welcome! Honza ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues 2018-02-21 15:44 [PATCH 0/2] fsnotify: Improve ENOMEM handling Jan Kara @ 2018-02-21 15:44 ` Jan Kara 2018-02-22 16:04 ` Amir Goldstein 2018-02-21 15:44 ` [PATCH 2/2] fsnotify: Let userspace know about lost events due to ENOMEM Jan Kara 1 sibling, 1 reply; 9+ messages in thread From: Jan Kara @ 2018-02-21 15:44 UTC (permalink / raw) To: linux-fsdevel; +Cc: Shakeel Butt, Amir Goldstein, Jan Kara Fanotify queues of unlimited length do not expect events can be lost. Since these queues are used for system auditing and other security related tasks, loosing events can even have security implications. So avoid loosing events due to failure to allocate memory by making event allocation use __GFP_NOFAIL. Since the allocation is small (32-bytes), currently there is no practical difference of this change but still it is good to have the expectation explicitely documented. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/notify/fanotify/fanotify.c | 19 ++++++++++++++----- fs/notify/fanotify/fanotify.h | 3 ++- fs/notify/fanotify/fanotify_user.c | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index 6702a6a0bbb5..928f2a5eedb7 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -139,23 +139,32 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark, return false; } -struct fanotify_event_info *fanotify_alloc_event(struct inode *inode, u32 mask, +struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group, + struct inode *inode, u32 mask, const struct path *path) { struct fanotify_event_info *event; + gfp_t gfp = GFP_KERNEL; + + /* + * For queues with unlimited length lost events are not expected and + * can possibly have security implications. Avoid losing events when + * memory is short. + */ + if (group->max_events == UINT_MAX) + gfp |= __GFP_NOFAIL; if (fanotify_is_perm_event(mask)) { struct fanotify_perm_event_info *pevent; - pevent = kmem_cache_alloc(fanotify_perm_event_cachep, - GFP_KERNEL); + pevent = kmem_cache_alloc(fanotify_perm_event_cachep, gfp); if (!pevent) return NULL; event = &pevent->fae; pevent->response = 0; goto init; } - event = kmem_cache_alloc(fanotify_event_cachep, GFP_KERNEL); + event = kmem_cache_alloc(fanotify_event_cachep, gfp); if (!event) return NULL; init: __maybe_unused @@ -210,7 +219,7 @@ static int fanotify_handle_event(struct fsnotify_group *group, return 0; } - event = fanotify_alloc_event(inode, mask, data); + event = fanotify_alloc_event(group, inode, mask, data); ret = -ENOMEM; if (unlikely(!event)) goto finish; diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h index 256d9d1ddea9..8609ba06f474 100644 --- a/fs/notify/fanotify/fanotify.h +++ b/fs/notify/fanotify/fanotify.h @@ -52,5 +52,6 @@ static inline struct fanotify_event_info *FANOTIFY_E(struct fsnotify_event *fse) return container_of(fse, struct fanotify_event_info, fse); } -struct fanotify_event_info *fanotify_alloc_event(struct inode *inode, u32 mask, +struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group, + struct inode *inode, u32 mask, const struct path *path); diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index c07eb3d655ea..72e367822efb 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -757,7 +757,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags) group->fanotify_data.user = user; atomic_inc(&user->fanotify_listeners); - oevent = fanotify_alloc_event(NULL, FS_Q_OVERFLOW, NULL); + oevent = fanotify_alloc_event(group, NULL, FS_Q_OVERFLOW, NULL); if (unlikely(!oevent)) { fd = -ENOMEM; goto out_destroy_group; -- 2.13.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues 2018-02-21 15:44 ` [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues Jan Kara @ 2018-02-22 16:04 ` Amir Goldstein 2018-02-22 16:22 ` Jan Kara 0 siblings, 1 reply; 9+ messages in thread From: Amir Goldstein @ 2018-02-22 16:04 UTC (permalink / raw) To: Jan Kara; +Cc: linux-fsdevel, Shakeel Butt On Wed, Feb 21, 2018 at 5:44 PM, Jan Kara <jack@suse.cz> wrote: > Fanotify queues of unlimited length do not expect events can be lost. > Since these queues are used for system auditing and other security Change looks good to me, but the reasoning is going backwards. IMO, you should mention that we are going to change -ENOMEM behavior to result in Q_OVERFLOW and user does not expect Q_OVERFLOW from an UNLIMITED queue. > related tasks, loosing events can even have security implications. So > avoid loosing events due to failure to allocate memory by making event > allocation use __GFP_NOFAIL. Since the allocation is small (32-bytes), > currently there is no practical difference of this change but still it > is good to have the expectation explicitely documented. > But if currently allocations cannot fail, then why do we need patch 2/2 (queue overflow event). It is because small allocations can fail when accounted to non-root memcg? Thanks, Amir. > Signed-off-by: Jan Kara <jack@suse.cz> > --- > fs/notify/fanotify/fanotify.c | 19 ++++++++++++++----- > fs/notify/fanotify/fanotify.h | 3 ++- > fs/notify/fanotify/fanotify_user.c | 2 +- > 3 files changed, 17 insertions(+), 7 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c > index 6702a6a0bbb5..928f2a5eedb7 100644 > --- a/fs/notify/fanotify/fanotify.c > +++ b/fs/notify/fanotify/fanotify.c > @@ -139,23 +139,32 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark, > return false; > } > > -struct fanotify_event_info *fanotify_alloc_event(struct inode *inode, u32 mask, > +struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group, > + struct inode *inode, u32 mask, > const struct path *path) > { > struct fanotify_event_info *event; > + gfp_t gfp = GFP_KERNEL; > + > + /* > + * For queues with unlimited length lost events are not expected and > + * can possibly have security implications. Avoid losing events when > + * memory is short. > + */ > + if (group->max_events == UINT_MAX) > + gfp |= __GFP_NOFAIL; > > if (fanotify_is_perm_event(mask)) { > struct fanotify_perm_event_info *pevent; > > - pevent = kmem_cache_alloc(fanotify_perm_event_cachep, > - GFP_KERNEL); > + pevent = kmem_cache_alloc(fanotify_perm_event_cachep, gfp); > if (!pevent) > return NULL; > event = &pevent->fae; > pevent->response = 0; > goto init; > } > - event = kmem_cache_alloc(fanotify_event_cachep, GFP_KERNEL); > + event = kmem_cache_alloc(fanotify_event_cachep, gfp); > if (!event) > return NULL; > init: __maybe_unused > @@ -210,7 +219,7 @@ static int fanotify_handle_event(struct fsnotify_group *group, > return 0; > } > > - event = fanotify_alloc_event(inode, mask, data); > + event = fanotify_alloc_event(group, inode, mask, data); > ret = -ENOMEM; > if (unlikely(!event)) > goto finish; > diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h > index 256d9d1ddea9..8609ba06f474 100644 > --- a/fs/notify/fanotify/fanotify.h > +++ b/fs/notify/fanotify/fanotify.h > @@ -52,5 +52,6 @@ static inline struct fanotify_event_info *FANOTIFY_E(struct fsnotify_event *fse) > return container_of(fse, struct fanotify_event_info, fse); > } > > -struct fanotify_event_info *fanotify_alloc_event(struct inode *inode, u32 mask, > +struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group, > + struct inode *inode, u32 mask, > const struct path *path); > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index c07eb3d655ea..72e367822efb 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -757,7 +757,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags) > group->fanotify_data.user = user; > atomic_inc(&user->fanotify_listeners); > > - oevent = fanotify_alloc_event(NULL, FS_Q_OVERFLOW, NULL); > + oevent = fanotify_alloc_event(group, NULL, FS_Q_OVERFLOW, NULL); > if (unlikely(!oevent)) { > fd = -ENOMEM; > goto out_destroy_group; > -- > 2.13.6 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues 2018-02-22 16:04 ` Amir Goldstein @ 2018-02-22 16:22 ` Jan Kara 2018-02-22 18:34 ` Amir Goldstein 0 siblings, 1 reply; 9+ messages in thread From: Jan Kara @ 2018-02-22 16:22 UTC (permalink / raw) To: Amir Goldstein; +Cc: Jan Kara, linux-fsdevel, Shakeel Butt On Thu 22-02-18 18:04:36, Amir Goldstein wrote: > On Wed, Feb 21, 2018 at 5:44 PM, Jan Kara <jack@suse.cz> wrote: > > Fanotify queues of unlimited length do not expect events can be lost. > > Since these queues are used for system auditing and other security > > Change looks good to me, but the reasoning is going backwards. > IMO, you should mention that we are going to change -ENOMEM > behavior to result in Q_OVERFLOW and user does not expect > Q_OVERFLOW from an UNLIMITED queue. See below. > > related tasks, loosing events can even have security implications. So > > avoid loosing events due to failure to allocate memory by making event > > allocation use __GFP_NOFAIL. Since the allocation is small (32-bytes), > > currently there is no practical difference of this change but still it > > is good to have the expectation explicitely documented. > > > > But if currently allocations cannot fail, then why do we need patch > 2/2 (queue overflow event). It is because small allocations can fail when > accounted to non-root memcg? Yes. So how about changelog like: Fanotify queues of unlimited length do not expect events can be lost. Since these queues are used for system auditing and other security related tasks, loosing events can even have security implications. Currently, since the allocation is small (32-bytes), it cannot fail however when we start accounting events in memcgs, allocation can start failing. So avoid loosing events due to failure to allocate memory by making event allocation use __GFP_NOFAIL. Thanks for review! Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues 2018-02-22 16:22 ` Jan Kara @ 2018-02-22 18:34 ` Amir Goldstein 2018-02-26 13:06 ` Jan Kara 0 siblings, 1 reply; 9+ messages in thread From: Amir Goldstein @ 2018-02-22 18:34 UTC (permalink / raw) To: Jan Kara; +Cc: linux-fsdevel, Shakeel Butt On Thu, Feb 22, 2018 at 6:22 PM, Jan Kara <jack@suse.cz> wrote: > On Thu 22-02-18 18:04:36, Amir Goldstein wrote: >> On Wed, Feb 21, 2018 at 5:44 PM, Jan Kara <jack@suse.cz> wrote: >> > Fanotify queues of unlimited length do not expect events can be lost. >> > Since these queues are used for system auditing and other security >> >> Change looks good to me, but the reasoning is going backwards. >> IMO, you should mention that we are going to change -ENOMEM >> behavior to result in Q_OVERFLOW and user does not expect >> Q_OVERFLOW from an UNLIMITED queue. > > See below. > >> > related tasks, loosing events can even have security implications. So >> > avoid loosing events due to failure to allocate memory by making event >> > allocation use __GFP_NOFAIL. Since the allocation is small (32-bytes), >> > currently there is no practical difference of this change but still it >> > is good to have the expectation explicitely documented. >> > >> >> But if currently allocations cannot fail, then why do we need patch >> 2/2 (queue overflow event). It is because small allocations can fail when >> accounted to non-root memcg? > > Yes. So how about changelog like: > > Fanotify queues of unlimited length do not expect events can be lost. > Since these queues are used for system auditing and other security > related tasks, loosing events can even have security implications. > Currently, since the allocation is small (32-bytes), it cannot fail > however when we start accounting events in memcgs, allocation can start > failing. So avoid loosing events due to failure to allocate memory by > making event allocation use __GFP_NOFAIL. > Very good. Thanks, Amir. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues 2018-02-22 18:34 ` Amir Goldstein @ 2018-02-26 13:06 ` Jan Kara 2018-02-26 13:17 ` Amir Goldstein 0 siblings, 1 reply; 9+ messages in thread From: Jan Kara @ 2018-02-26 13:06 UTC (permalink / raw) To: Amir Goldstein; +Cc: Jan Kara, linux-fsdevel, Shakeel Butt On Thu 22-02-18 20:34:48, Amir Goldstein wrote: > On Thu, Feb 22, 2018 at 6:22 PM, Jan Kara <jack@suse.cz> wrote: > > On Thu 22-02-18 18:04:36, Amir Goldstein wrote: > >> On Wed, Feb 21, 2018 at 5:44 PM, Jan Kara <jack@suse.cz> wrote: > >> > Fanotify queues of unlimited length do not expect events can be lost. > >> > Since these queues are used for system auditing and other security > >> > >> Change looks good to me, but the reasoning is going backwards. > >> IMO, you should mention that we are going to change -ENOMEM > >> behavior to result in Q_OVERFLOW and user does not expect > >> Q_OVERFLOW from an UNLIMITED queue. > > > > See below. > > > >> > related tasks, loosing events can even have security implications. So > >> > avoid loosing events due to failure to allocate memory by making event > >> > allocation use __GFP_NOFAIL. Since the allocation is small (32-bytes), > >> > currently there is no practical difference of this change but still it > >> > is good to have the expectation explicitely documented. > >> > > >> > >> But if currently allocations cannot fail, then why do we need patch > >> 2/2 (queue overflow event). It is because small allocations can fail when > >> accounted to non-root memcg? > > > > Yes. So how about changelog like: > > > > Fanotify queues of unlimited length do not expect events can be lost. > > Since these queues are used for system auditing and other security > > related tasks, loosing events can even have security implications. > > Currently, since the allocation is small (32-bytes), it cannot fail > > however when we start accounting events in memcgs, allocation can start > > failing. So avoid loosing events due to failure to allocate memory by > > making event allocation use __GFP_NOFAIL. > > > > Very good. Can I add your reviewed-by to the patch then? Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues 2018-02-26 13:06 ` Jan Kara @ 2018-02-26 13:17 ` Amir Goldstein 0 siblings, 0 replies; 9+ messages in thread From: Amir Goldstein @ 2018-02-26 13:17 UTC (permalink / raw) To: Jan Kara; +Cc: linux-fsdevel, Shakeel Butt On Mon, Feb 26, 2018 at 3:06 PM, Jan Kara <jack@suse.cz> wrote: > On Thu 22-02-18 20:34:48, Amir Goldstein wrote: >> On Thu, Feb 22, 2018 at 6:22 PM, Jan Kara <jack@suse.cz> wrote: >> > On Thu 22-02-18 18:04:36, Amir Goldstein wrote: >> >> On Wed, Feb 21, 2018 at 5:44 PM, Jan Kara <jack@suse.cz> wrote: >> >> > Fanotify queues of unlimited length do not expect events can be lost. >> >> > Since these queues are used for system auditing and other security >> >> >> >> Change looks good to me, but the reasoning is going backwards. >> >> IMO, you should mention that we are going to change -ENOMEM >> >> behavior to result in Q_OVERFLOW and user does not expect >> >> Q_OVERFLOW from an UNLIMITED queue. >> > >> > See below. >> > >> >> > related tasks, loosing events can even have security implications. So >> >> > avoid loosing events due to failure to allocate memory by making event >> >> > allocation use __GFP_NOFAIL. Since the allocation is small (32-bytes), >> >> > currently there is no practical difference of this change but still it >> >> > is good to have the expectation explicitely documented. >> >> > >> >> >> >> But if currently allocations cannot fail, then why do we need patch >> >> 2/2 (queue overflow event). It is because small allocations can fail when >> >> accounted to non-root memcg? >> > >> > Yes. So how about changelog like: >> > >> > Fanotify queues of unlimited length do not expect events can be lost. >> > Since these queues are used for system auditing and other security >> > related tasks, loosing events can even have security implications. >> > Currently, since the allocation is small (32-bytes), it cannot fail >> > however when we start accounting events in memcgs, allocation can start >> > failing. So avoid loosing events due to failure to allocate memory by >> > making event allocation use __GFP_NOFAIL. >> > >> >> Very good. > > Can I add your reviewed-by to the patch then? > Yes, of course. Thanks, Amir. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] fsnotify: Let userspace know about lost events due to ENOMEM 2018-02-21 15:44 [PATCH 0/2] fsnotify: Improve ENOMEM handling Jan Kara 2018-02-21 15:44 ` [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues Jan Kara @ 2018-02-21 15:44 ` Jan Kara 2018-02-22 16:06 ` Amir Goldstein 1 sibling, 1 reply; 9+ messages in thread From: Jan Kara @ 2018-02-21 15:44 UTC (permalink / raw) To: linux-fsdevel; +Cc: Shakeel Butt, Amir Goldstein, Jan Kara 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@suse.cz> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] fsnotify: Let userspace know about lost events due to ENOMEM 2018-02-21 15:44 ` [PATCH 2/2] fsnotify: Let userspace know about lost events due to ENOMEM Jan Kara @ 2018-02-22 16:06 ` Amir Goldstein 0 siblings, 0 replies; 9+ messages in thread From: Amir Goldstein @ 2018-02-22 16:06 UTC (permalink / raw) To: Jan Kara; +Cc: linux-fsdevel, Shakeel Butt On Wed, Feb 21, 2018 at 5:44 PM, Jan Kara <jack@suse.cz> 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@suse.cz> Reviewed-by: Amir Goldstein <amir73il@gmail.com> > --- > 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-02-26 13:17 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-21 15:44 [PATCH 0/2] fsnotify: Improve ENOMEM handling Jan Kara 2018-02-21 15:44 ` [PATCH 1/2] fanotify: Avoid lost events due to ENOMEM for unlimited queues Jan Kara 2018-02-22 16:04 ` Amir Goldstein 2018-02-22 16:22 ` Jan Kara 2018-02-22 18:34 ` Amir Goldstein 2018-02-26 13:06 ` Jan Kara 2018-02-26 13:17 ` Amir Goldstein 2018-02-21 15:44 ` [PATCH 2/2] fsnotify: Let userspace know about lost events due to ENOMEM Jan Kara 2018-02-22 16:06 ` Amir Goldstein
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).