Re: [PATCH v4 3/3] fanotify: introduce new event type FAN_OPEN_EXEC_PERM

[Matthew Bobrowski <mbobrowski@mbobrowski.org>]

On Tue, Oct 16, 2018 at 02:04:20PM +0200, Jan Kara wrote:
> On Thu 11-10-18 21:43:09, Matthew Bobrowski wrote:
> > A new event type FAN_OPEN_EXEC_PERM has been defined. This allows users
> > to receive and grant access to files that are intending to be opened for
> > execution.
> 
> The same note here as for FAN_OPEN_EXEC.

Updated.

I will include the updates to this within the updated patch series.

> > This acts in the same manor as previous permission event types, meaning
> > that a access response is required from the application to permit
> > further operations on the file. The event flag is set specifically when
> > the __FMODE_EXEC flag is set within file->f_flags in the fsnotify_perm()
> > hook.
> > 
> > Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> 
> And a general question: Do I understand right that your audit application
> needs this event type?  

Yes, your understanding is correct. The application we're developing
requires this event type.

> I'm asking because so far what Steve wrote me was about pattern matching
> of fanotify events to detect when something happens. So it's not clear
> to me how permission event fits into that.

That detection for when "something happens", is precisely to do with
understanding when a file is opened for execution. We use permission
events within the application we're developing to either ALLOW or DENY a
particular action being performed against an object. At the moment, we
don't have any way to determine what actually caused a watched object to
be opened, so this is to aid with that, and also help us make a better
judgment how we're to respond to a particular action.

> > ---
> >  fs/notify/fanotify/fanotify.c    |  3 ++-
> >  fs/notify/fsnotify.c             |  2 +-
> >  include/linux/fanotify.h         |  3 ++-
> >  include/linux/fsnotify.h         | 10 ++++++----
> >  include/linux/fsnotify_backend.h |  7 ++++---
> >  include/uapi/linux/fanotify.h    |  1 +
> >  6 files changed, 16 insertions(+), 10 deletions(-)
> > 
> > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > index 9da334d343b8..bae9a312d772 100644
> > --- a/fs/notify/fanotify/fanotify.c
> > +++ b/fs/notify/fanotify/fanotify.c
> > @@ -212,8 +212,9 @@ static int fanotify_handle_event(struct fsnotify_group *group,
> >  	BUILD_BUG_ON(FAN_ACCESS_PERM != FS_ACCESS_PERM);
> >  	BUILD_BUG_ON(FAN_ONDIR != FS_ISDIR);
> >  	BUILD_BUG_ON(FAN_OPEN_EXEC != FS_OPEN_EXEC);
> > +	BUILD_BUG_ON(FAN_OPEN_EXEC_PERM != FS_OPEN_EXEC_PERM);
> >  
> > -	BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 11);
> > +	BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 12);
> >  
> >  	event_mask = fanotify_group_event_mask(iter_info, mask, data,
> >  						data_type);
> > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> > index 051e5fc0dba1..40ed97aede29 100644
> > --- a/fs/notify/fsnotify.c
> > +++ b/fs/notify/fsnotify.c
> > @@ -402,7 +402,7 @@ static __init int fsnotify_init(void)
> >  {
> >  	int ret;
> >  
> > -	BUILD_BUG_ON(HWEIGHT32(ALL_FSNOTIFY_BITS) != 24);
> > +	BUILD_BUG_ON(HWEIGHT32(ALL_FSNOTIFY_BITS) != 25);
> >  
> >  	ret = init_srcu_struct(&fsnotify_mark_srcu);
> >  	if (ret)
> > diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> > index 0e07d23b6c17..614943153596 100644
> > --- a/include/linux/fanotify.h
> > +++ b/include/linux/fanotify.h
> > @@ -42,7 +42,8 @@
> >  				 FAN_CLOSE | FAN_OPEN | FAN_OPEN_EXEC)
> >  
> >  /* Events that require a permission response from user */
> > -#define FANOTIFY_PERM_EVENTS	(FAN_OPEN_PERM | FAN_ACCESS_PERM)
> > +#define FANOTIFY_PERM_EVENTS	(FAN_OPEN_PERM | FAN_ACCESS_PERM | \
> > +				 FAN_OPEN_EXEC_PERM)
> >  
> >  /* Extra flags that may be reported with event or control handling of events */
> >  #define FANOTIFY_EVENT_FLAGS	(FAN_EVENT_ON_CHILD | FAN_ONDIR)
> > diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> > index 1fe5ac93b252..6fb8d1fcfeaf 100644
> > --- a/include/linux/fsnotify.h
> > +++ b/include/linux/fsnotify.h
> > @@ -38,12 +38,14 @@ static inline int fsnotify_perm(struct file *file, int mask)
> >  		return 0;
> >  	if (!(mask & (MAY_READ | MAY_OPEN)))
> >  		return 0;
> > -	if (mask & MAY_OPEN)
> > +	if (mask & MAY_OPEN) {
> >  		fsnotify_mask = FS_OPEN_PERM;
> > -	else if (mask & MAY_READ)
> > +
> > +		if (file->f_flags & __FMODE_EXEC)
> > +			fsnotify_mask |= FS_OPEN_EXEC_PERM;
> > +	} else if (mask & MAY_READ) {
> >  		fsnotify_mask = FS_ACCESS_PERM;
> > -	else
> > -		BUG();
> > +	}
> >  
> >  	ret = fsnotify_parent(path, NULL, fsnotify_mask);
> >  	if (ret)
> > diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> > index 329ac6684326..96616651220c 100644
> > --- a/include/linux/fsnotify_backend.h
> > +++ b/include/linux/fsnotify_backend.h
> > @@ -44,8 +44,9 @@
> >  #define FS_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
> >  #define FS_IN_IGNORED		0x00008000	/* last inotify event here */
> >  
> > -#define FS_OPEN_PERM		0x00010000	/* open event in an permission hook */
> > +#define FS_OPEN_PERM		0x00010000	/* open event in a permission hook */
> >  #define FS_ACCESS_PERM		0x00020000	/* access event in a permissions hook */
> > +#define FS_OPEN_EXEC_PERM	0x00040000	/* open/exec in a permission hook */
> >  
> >  #define FS_EXCL_UNLINK		0x04000000	/* do not send events if object is unlinked */
> >  #define FS_ISDIR		0x40000000	/* event occurred against dir */
> > @@ -64,7 +65,7 @@
> >  				   FS_CLOSE_WRITE | FS_CLOSE_NOWRITE | FS_OPEN |\
> >  				   FS_MOVED_FROM | FS_MOVED_TO | FS_CREATE |\
> >  				   FS_DELETE | FS_OPEN_PERM | FS_ACCESS_PERM |\
> > -				   FS_OPEN_EXEC)
> > +				   FS_OPEN_EXEC | FS_OPEN_EXEC_PERM)
> >  
> >  #define FS_MOVE			(FS_MOVED_FROM | FS_MOVED_TO)
> >  
> > @@ -77,7 +78,7 @@
> >  			     FS_DELETE | FS_DELETE_SELF | FS_MOVE_SELF | \
> >  			     FS_UNMOUNT | FS_Q_OVERFLOW | FS_IN_IGNORED | \
> >  			     FS_OPEN_PERM | FS_ACCESS_PERM | FS_DN_RENAME |\
> > -			     FS_OPEN_EXEC)
> > +			     FS_OPEN_EXEC | FS_OPEN_EXEC_PERM)
> >  
> >  /* Extra flags that may be reported with event or control handling of events */
> >  #define ALL_FSNOTIFY_FLAGS  (FS_EXCL_UNLINK | FS_ISDIR | FS_IN_ONESHOT | \
> > diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> > index da278f11ab29..e2df10c61cb1 100644
> > --- a/include/uapi/linux/fanotify.h
> > +++ b/include/uapi/linux/fanotify.h
> > @@ -16,6 +16,7 @@
> >  
> >  #define FAN_OPEN_PERM		0x00010000	/* File open in perm check */
> >  #define FAN_ACCESS_PERM		0x00020000	/* File accessed in perm check */
> > +#define FAN_OPEN_EXEC_PERM	0x00040000	/* File open/exec in perm check */
> >  
> >  #define FAN_ONDIR		0x40000000	/* event occurred against dir */
> >  
> > -- 
> > 2.17.2
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR