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=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT 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 EE5D7C43381 for ; Mon, 1 Apr 2019 17:52:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C934320830 for ; Mon, 1 Apr 2019 17:52:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732508AbfDARw6 (ORCPT ); Mon, 1 Apr 2019 13:52:58 -0400 Received: from mx2.suse.de ([195.135.220.15]:50072 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1732538AbfDAR0Y (ORCPT ); Mon, 1 Apr 2019 13:26:24 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id B10A2ACC1; Mon, 1 Apr 2019 17:26:22 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 588091E42C7; Mon, 1 Apr 2019 19:26:22 +0200 (CEST) Date: Mon, 1 Apr 2019 19:26:22 +0200 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , linux-fsdevel , mhocko@suse.cz, Al Viro , Marko Rauhamaa Subject: Re: fanotify permission events on virtual filesystem Message-ID: <20190401172622.GB19542@quack2.suse.cz> References: <20190320131642.GE9485@quack2.suse.cz> <20190320143048.GH9485@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Wed 20-03-19 17:02:54, Amir Goldstein wrote: > On Wed, Mar 20, 2019 at 4:30 PM Jan Kara wrote: > > > > On Wed 20-03-19 15:46:20, Amir Goldstein wrote: > > > On Wed, Mar 20, 2019 at 3:16 PM Jan Kara wrote: > > > > recently, one of our customers has reported a deadlock with fanotify. The > > > > analysis has shown that a process has put (likely by mistake) FAN_OPEN_PERM > > > > mark on /proc and / filesystem. That resulted in a deadlock like follows: > > > > > > > > process 1: process 2: process 3: > > > > open("/proc/process 2/maps") > > > > - blocks waiting for response to > > > > FAN_OPEN_PERM event > > > > > > > > exec(2) > > > > __do_execve_file() > > > > - grabs current->signal->cred_guard_mutex > > > > do_open_execat() > > > > - blocks waiting for response to > > > > FAN_OPEN_PERM event > > > > > > > > reads fanotify event > > > > generated by process 1 > > > > create_fd() > > > > dentry_open() > > > > proc_maps_open() > > > > blocks on > > > > cred_guard_mutex of process 2. > > > > > > > > Now this is not the only case where you can setup fanotify permissions > > > > events so that your listener deadlocks but I'd argue that this case is > > > > especially nasty and it is unrealistic to expect from userspace that it > > > > would be able to implement fanotify listener in such a way that is > > > > deadlock-free for proc filesystem since the lock dependencies there are > > > > very different. So how about we just forbid placing marks with fanotify > > > > permission events on proc? Any other virtual filesystem we should exclude? > > > > > > > > > > I bet if we forbid placing marks on /proc, some apps would break. > > > > Well, I didn't mean all marks, just the permission ones. I'm not sure there > > are apps that place permission events on /proc... > > > > Maybe not intentionally. > I once tested a few fanotify based AntiVirus solutions. > In some of them, setting an "Exclude path" on some mount point > would cause mark to not be set on that path, but for one in particular, > the mark was still being set on the mount so path pattern filtering was > done after receiving the events. > I did not check whether /proc was blacklisted out of the box or if it > could be marked/excluded from scan. > IMO, assuming that all AntiVirus vendors blacklist all virtual filesystems > is an assumption that we need to validate. > [CC Marko from F-Secure for commenting on the above.] > > > > I always though that allowing O_PATH in event_f_flags can make > > > sense for some apps. > > > > > > What if instead of forbiding marks of /proc, we would force those > > > marks to use O_PATH for fd creation. Some of the functionality > > > will remain. Apps are less likely to break. Deadlocks will be less > > > likely, although maybe still possible. > > > > Yes, that's another option. But if this is automatic, it is going to be > > confusing to potential users - report O_PATH fd if getting normal fd is > > dangerous isn't great. And also the deadlocks are there only for permission > > events so there's no strong reason to restrict normal ones. > > > > Of course, we can do this hack only for reading permission events > for virtual filesystems. The question is what would be more surprising > to existing apps: > - marks for permission events no longer working on /proc > - fd from permission events no longer readable from /proc > - something completely different I think that option 1) is at least very explicit failure and given what Marko wrote I'm willing to give that a try. If people start complaining, we'll probably have to revert and try either reporting O_PATH fd or even FAN_NOFD. It's a bit of a gamble I agree but current status quo isn't good either. Honza -- Jan Kara SUSE Labs, CR