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>
> 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
>
