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 27CE3C43381 for ; Wed, 20 Mar 2019 14:30:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 020A5218A2 for ; Wed, 20 Mar 2019 14:30:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727750AbfCTOav (ORCPT ); Wed, 20 Mar 2019 10:30:51 -0400 Received: from mx2.suse.de ([195.135.220.15]:34052 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726169AbfCTOau (ORCPT ); Wed, 20 Mar 2019 10:30:50 -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 12CF7AF52; Wed, 20 Mar 2019 14:30:49 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id B47511E4255; Wed, 20 Mar 2019 15:30:48 +0100 (CET) Date: Wed, 20 Mar 2019 15:30:48 +0100 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , linux-fsdevel , mhocko@suse.cz, Al Viro Subject: Re: fanotify permission events on virtual filesystem Message-ID: <20190320143048.GH9485@quack2.suse.cz> References: <20190320131642.GE9485@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 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... > 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. > Note that the new FAN_REPORT_FID listener already excludes > marking most virtual filesystems for lack of s_export_op. Yes, I know. Honza -- Jan Kara SUSE Labs, CR