linux-kernel - Re: soft lockup in fanotify

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200714131413.GJ23073@quack2.suse.cz>
Date:   Tue, 14 Jul 2020 15:14:13 +0200
From:   Jan Kara <jack@...e.cz>
To:     Francesco Ruggeri <fruggeri@...sta.com>
Cc:     linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        amir73il@...il.com, jack@...e.cz
Subject: Re: soft lockup in fanotify_read

Hello!

On Mon 13-07-20 19:54:17, Francesco Ruggeri wrote:
> We are getting this soft lockup in fanotify_read.
> The reason is that this code does not seem to scale to cases where there
> are big bursts of events generated by fanotify_handle_event.
> fanotify_read acquires group->notification_lock for each event.
> fanotify_handle_event uses the lock to add one event, which also involves
> fanotify_merge, which scans the whole list trying to find an event to
> merge the new one with.
> In our case fanotify_read is invoked with a buffer big enough for 200
> events, and what happens is that every time fanotify_read dequeues an
> event and releases the lock, fanotify_handle_event adds several more,
> scanning a longer and longer list. This causes fanotify_read to wait
> longer and longer for the lock, and the soft lockup happens before
> fanotify_read can reach 200 events.
> Is it intentional for fanotify_read to acquire the lock for each event,
> rather than batching together a user buffer worth of events?

Thanks for report and the analysis. I agree what you describe is possible.
The locking is actually fine I think but you're correct that the merging
logic isn't ideal and for large amounts of queued events may be too slow.
We were already discussing with Amir how to speed it up but didn't end up
doing anything yet since the issue wasn't really pressing.

WRT fanotify_read() removing events from the list in batches: That's
certainly one possible optimization but (especially with recent extensions
to fanotify interface) it is difficult to tell how many events will
actually fit in the provided buffer so we'd have to provide a way to push
events back to the event queue which may get a bit tricky. And as I wrote
above I think the real problem is actually with fanotify merge logic which
ends up holding the notification_lock for too long...

But we may want to add cond_resched() into the loop in fanotify_read() as
that can currently take time proportinal to user-provided buffer which can
be a lot. That will probably silence the softlockup for you as well
(although it's not really fixing the underlying issue).

We'll have a look what we can do about this :)

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR