On Wed 15-07-20 15:34:48, Amir Goldstein wrote:
> On Wed, Jul 15, 2020 at 3:29 PM Jan Kara <jack@xxxxxxx> wrote:
> >
> > When user provides large buffer for events and there are lots of events
> > available, we can try to copy them all to userspace without scheduling
> > which can softlockup the kernel (furthermore exacerbated by the
> > contention on notification_lock). Add a scheduling point after copying
> > each event.
> >
> > Note that usually the real underlying problem is the cost of fanotify
> > event merging and the resulting contention on notification_lock but this
> > is a cheap way to somewhat reduce the problem until we can properly
> > address that.
> >
> > Reported-by: Francesco Ruggeri <fruggeri@xxxxxxxxxx>
> > Signed-off-by: Jan Kara <jack@xxxxxxx>
> > ---
> > fs/notify/fanotify/fanotify_user.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > This is a quick mending we can do immediately and is probably a good idea
> > nevertheless... I'll queue it up if Amir agrees.
> >
>
> Sure. fine by me.
> Maybe add a lore link to the issue report.
>
> Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
Thanks. I've added the link and pushed out the patch to my tree.
Honza
>
> > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> > index 63b5dffdca9e..d7f63aeca992 100644
> > --- a/fs/notify/fanotify/fanotify_user.c
> > +++ b/fs/notify/fanotify/fanotify_user.c
> > @@ -412,6 +412,11 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
> >
> > add_wait_queue(&group->notification_waitq, &wait);
> > while (1) {
> > + /*
> > + * User can supply arbitrarily large buffer. Avoid softlockups
> > + * in case there are lots of available events.
> > + */
> > + cond_resched();
> > event = get_one_event(group, count);
> > if (IS_ERR(event)) {
> > ret = PTR_ERR(event);
> > --
> > 2.16.4
> >
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
