From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-f179.google.com ([209.85.214.179]:42908 "EHLO mail-pl1-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726138AbeKGHup (ORCPT ); Wed, 7 Nov 2018 02:50:45 -0500 Received: by mail-pl1-f179.google.com with SMTP id t6-v6so6874693plo.9 for ; Tue, 06 Nov 2018 14:23:19 -0800 (PST) Date: Wed, 7 Nov 2018 09:23:12 +1100 From: Matthew Bobrowski To: Amir Goldstein Cc: Jan Kara , linux-fsdevel Subject: Re: FAN_OPEN_EXEC event and ignore mask Message-ID: <20181106222310.GA31364@lithium.mbobrowski.org> References: <20181031103939.GA8325@development.internal.lab> <20181102125000.GB15086@quack2.suse.cz> <20181103003411.GA1314@development.internal.lab> <20181105084147.GC6953@quack2.suse.cz> <20181106130837.GA2206@development.internal.lab> <20181106203952.GA1726@development.internal.lab> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Nov 06, 2018 at 11:15:11PM +0200, Amir Goldstein wrote: > > > > > > IDGI. What is the problem with: > > > > > > if (mask & MAY_OPEN) { > > > fsnotify_mask = FS_OPEN_PERM; > > > if (file->f_flags & __FMODE_EXEC) { > > > ret = fsnotify_path(inode, path, FS_OPEN_EXEC_PERM); > > > if (ret) return ret; > > > } > > > } else if (mask & MAY_READ) { > > > fsnotify_mask = FS_ACCESS_PERM; > > > } > > > > > > return fsnotify_path(inode, path, FS_OPEN_EXEC_PERM); > > > > > > You can consolidate all 5 calls to fsnotify_parent();fsnotify() of the same > > > pattern to fsnotify_path(). > > > > There is nothing wrong with this and what this does in fact simplifies > > the call site for fsnotify_parent()/fsnotify(), which is very nice and > > clean in my opinion. > > > > What I'm referring to though is different. All I'm saying is that if I was > > a user and I wanted to capture each time a file was opened regardless > > whether it was for execution, for read, for write, I'd expect to capture > > these events by just registering for FAN_OPEN_PERM and it would be > > sufficient. After applying these updates, for a user to capture *all* open > > related events, they're going to have to now supply both FAN_OPEN_PERM and > > FAN_OPEN_EXEC_PERM. I just don't want to be in a position where we've > > completely changed the expectation of FAN_OPEN_PERM, as I can imagine this > > would really frustrate people. > > > > Maybe I'm over thinking it and it's OK? > > > > I don't know if you are overthinking but I still don't understand the concern. > > Before the change: > - Listen on FAN_OPEN_PERM > - open file for read > - open file for write > - execve() > User gets 3 FAN_OPEN_PERM permission events > > After the change this is still the behavior with or without requesting > FAN_OPEN_EXEC_PERM. > > It's quite obvious that before the change user cannot request > FAN_OPEN_EXEC_PERM and after the change requesting > FAN_OPEN_EXEC_PERM with or without FAN_OPEN_PERM, > will result in getting the new event once. > > The only case where user won't get 3 FAN_OPEN_PERM events > is when user denies the FAN_OPEN_EXEC_PERM event. > What is your concern? OK, my apologies. It was the way I implemented it that was causing me to get confused as the results weren't aligning with what you so rightfully outlined above. Sorry about that Amir. -- Matthew Bobrowski