From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E96CC0044B for ; Wed, 13 Feb 2019 21:02:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0E900207E0 for ; Wed, 13 Feb 2019 21:02:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="HqgWAVHI" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387502AbfBMVCa (ORCPT ); Wed, 13 Feb 2019 16:02:30 -0500 Received: from mail-yb1-f194.google.com ([209.85.219.194]:45302 "EHLO mail-yb1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726189AbfBMVCa (ORCPT ); Wed, 13 Feb 2019 16:02:30 -0500 Received: by mail-yb1-f194.google.com with SMTP id e131so1490392ybh.12 for ; Wed, 13 Feb 2019 13:02:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=E0Zz289eR0JWdWhnihbiWWGD/mXDnDHA2x9N20iI7mE=; b=HqgWAVHIglyyBm1lcX7TfvorYT1LV0NwK0c7c34CCp5t6FX0UPXArYMMwHHl5/TOrO sKPJIcdZ8NdBXTBZcxFCKOB7TSNPlggxrIDFMZDBqGwi0yxL+SzDdZAfD1WNPKPjwrgk 81yclk0AmH0H34eGCkUv0jMB7vfKgGZpJ44dnVupaUaLatIphyBNumWOagt8nm+gOovy kML+PEq33rhePyOBRqo3n8xzbBXBwvQuy9QcYW45yRWZKN7tHi+NmbI07OxsuguX4n7q D3pjBcuUie6W8BsAFI21EFGSlYQkiTtrfcTeChBqBIKLzK2kKKqM9xKXVVQ8Bknc3b/2 iPeQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=E0Zz289eR0JWdWhnihbiWWGD/mXDnDHA2x9N20iI7mE=; b=oK8fKVidUTYE73btbx0yH3YDFmicqC3ACNYEenifJuIYIAzq2yrT5mbYQjSFIzCwh/ aOezuDlshojD8oJmsO1b0V7l4KEB4ltNGj4kZ5ZA8keGD7Iqi2OvsIpOwloUDzW+l6wv BiON23QGhuDsjLw11st7+VoOcgLuMVVwQct0aINeXOsWLx1mkDvPNv5dhZ1B7AlV5WzO +QnuE+QOsPsluql5BC58siglUUjPLnbLTeWeiOqA3MWCMladA3XnZEMG6gRc4C1LC5as l0R7SctHbZ3KN2rmutzXZw+2dlKIOfKwaqe1pkP0vqT7Vw9LlCrBo1xBxApzGEhho5Y0 htgQ== X-Gm-Message-State: AHQUAubGfGiV7/hUdB0Hzzn5Kqx3/sJNhJTif0LKgfgw1FMCJFBovN3O 42geNIMD4sZatLN0+2JRwqszxmvL8T1ZvaZmHWc= X-Google-Smtp-Source: AHgI3IaQfHC5LEkUKbjd+q6FvBAz5bioIqx2NYS/xf6+gYjsUrtRK2Ju614LZn2vVZp6KdV0C5Uhcj+R/8+hteWxg9U= X-Received: by 2002:a25:9703:: with SMTP id d3mr82176ybo.397.1550091748739; Wed, 13 Feb 2019 13:02:28 -0800 (PST) MIME-Version: 1.0 References: <20190213145443.26836-1-jack@suse.cz> <20190213145443.26836-7-jack@suse.cz> In-Reply-To: <20190213145443.26836-7-jack@suse.cz> From: Amir Goldstein Date: Wed, 13 Feb 2019 23:02:17 +0200 Message-ID: Subject: Re: [PATCH 6/6] fanotify: Use interruptible wait when waiting for permission events To: Jan Kara Cc: linux-fsdevel , Konstantin Khlebnikov , Vivek Trivedi , Orion Poplawski Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Wed, Feb 13, 2019 at 4:54 PM Jan Kara wrote: > > When waiting for response to fanotify permission events, we currently > use uninterruptible waits. That makes code simple however it can cause > lots of processes to end up in uninterruptible sleep with hard reboot > being the only alternative in case fanotify listener process stops > responding (e.g. due to a bug in its implementation). Uninterruptible > sleep also makes system hibernation fail if the listener gets frozen > before the process generating fanotify permission event. > > Fix these problems by using interruptible sleep for waiting for response > to fanotify event. This is slightly tricky though - we have to > detect when the event got already reported to userspace as in that > case we must not free the event. Instead we push the responsibility for > freeing the event to the process that will write response to the > event. > > Reported-by: Orion Poplawski > Reported-by: Konstantin Khlebnikov > Signed-off-by: Jan Kara > --- > fs/notify/fanotify/fanotify.c | 35 ++++++++++++++++++++++++++++++++--- > fs/notify/fanotify/fanotify.h | 3 ++- > 2 files changed, 34 insertions(+), 4 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c > index e725831be161..2e40d5d8660b 100644 > --- a/fs/notify/fanotify/fanotify.c > +++ b/fs/notify/fanotify/fanotify.c > @@ -57,6 +57,13 @@ static int fanotify_merge(struct list_head *list, struct fsnotify_event *event) > return 0; > } > > +/* > + * Wait for response to permission event. The function also takes care of > + * freeing the permission event (or offloads that in case the wait is canceled > + * by a signal). The function returns 0 in case access got allowed by userspace, > + * -EPERM in case userspace disallowed the access, and -ERESTARTSYS in case > + * the wait got interrupted by a signal. > + */ > static int fanotify_get_response(struct fsnotify_group *group, > struct fanotify_perm_event_info *event, > struct fsnotify_iter_info *iter_info) > @@ -65,8 +72,29 @@ static int fanotify_get_response(struct fsnotify_group *group, > > pr_debug("%s: group=%p event=%p\n", __func__, group, event); > > - wait_event(group->fanotify_data.access_waitq, > - event->state == FAN_EVENT_ANSWERED); > + ret = wait_event_interruptible(group->fanotify_data.access_waitq, > + event->state == FAN_EVENT_ANSWERED); > + /* Signal pending? */ > + if (ret < 0) { > + spin_lock(&group->notification_lock); > + /* Event reported to userspace and no answer yet? */ > + if (event->state == FAN_EVENT_REPORTED) { > + /* Event will get freed once userspace answers to it */ Did you forget to commit the if (event->state == FAN_EVENT_CANCELED) code? > + event->state = FAN_EVENT_CANCELED; > + spin_unlock(&group->notification_lock); > + return ret; > + } > + /* Event not yet reported? Just remove it. */ > + if (event->state == FAN_EVENT_INIT) > + fsnotify_remove_queued_event(group, &event->fae.fse); > + /* > + * Event may be also answered in case signal delivery raced > + * with wakeup. In that case we have nothing to do besides > + * freeing the event and reporting error. > + */ > + spin_unlock(&group->notification_lock); > + goto out; > + } > > /* userspace responded, convert to something usable */ > switch (event->response & ~FAN_AUDIT) { > @@ -84,6 +112,8 @@ static int fanotify_get_response(struct fsnotify_group *group, > > pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__, > group, event, ret); > +out: > + fsnotify_destroy_event(group, &event->fae.fse); > > return ret; > } > @@ -255,7 +285,6 @@ static int fanotify_handle_event(struct fsnotify_group *group, > } else if (fanotify_is_perm_event(mask)) { > ret = fanotify_get_response(group, FANOTIFY_PE(fsn_event), > iter_info); > - fsnotify_destroy_event(group, fsn_event); > } > finish: > if (fanotify_is_perm_event(mask)) > diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h > index 98d58939777c..bbdd2adfbf0f 100644 > --- a/fs/notify/fanotify/fanotify.h > +++ b/fs/notify/fanotify/fanotify.h > @@ -26,7 +26,8 @@ struct fanotify_event_info { > enum { > FAN_EVENT_INIT, > FAN_EVENT_REPORTED, > - FAN_EVENT_ANSWERED > + FAN_EVENT_ANSWERED, > + FAN_EVENT_CANCELED, > }; > > /* > -- > 2.16.4 >