On Thu, Dec 22, 2016 at 11:15 AM, Jan Kara <jack@xxxxxxx> wrote:
> When userspace task processing fanotify permission events screws up and
> does not respond, fsnotify_mark_srcu SRCU is held indefinitely which
> causes further hangs in the whole notification subsystem. Although we
> cannot easily solve the problem of operations blocked waiting for
> response from userspace, we can at least somewhat localize the damage by
> dropping SRCU lock before waiting for userspace response and reacquiring
> it when userspace responds.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
Looks good. one nit below.
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> fs/notify/fanotify/fanotify.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 2e8ca885fb3e..284d2d112ad2 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -61,7 +61,10 @@ static int fanotify_merge(struct list_head *list, struct fsnotify_event *event)
>
> #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
> static int fanotify_get_response(struct fsnotify_group *group,
> - struct fanotify_perm_event_info *event)
> + struct fsnotify_mark *inode_mark,
> + struct fsnotify_mark *vfsmount_mark,
> + struct fanotify_perm_event_info *event,
> + int *srcu_idx)
> {
> int ret;
>
> @@ -69,6 +72,15 @@ static int fanotify_get_response(struct fsnotify_group *group,
>
> wait_event(group->fanotify_data.access_waitq, event->response);
>
> + if (!fsnotify_prepare_user_wait(inode_mark, vfsmount_mark, srcu_idx)) {
Since it is not clear for reader of this code the conditions where
fsnotify_prepare_user_wait() can fail, a comment here would be nice
to explain the choice of ALLOW
> + event->response = FAN_ALLOW;
> + goto out;
> + }
> +
> + wait_event(group->fanotify_data.access_waitq, event->response);
> +
> + fsnotify_finish_user_wait(inode_mark, vfsmount_mark, srcu_idx);
> +out:
> /* userspace responded, convert to something usable */
> switch (event->response) {
> case FAN_ALLOW:
> @@ -220,7 +232,8 @@ static int fanotify_handle_event(struct fsnotify_group *group,
>
> #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
> if (mask & FAN_ALL_PERM_EVENTS) {
> - ret = fanotify_get_response(group, FANOTIFY_PE(fsn_event));
> + ret = fanotify_get_response(group, inode_mark, fanotify_mark,
> + FANOTIFY_PE(fsn_event), srcu_idx);
> fsnotify_destroy_event(group, fsn_event);
> }
> #endif
> --
> 2.10.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
