[LTP] [PATCH 4/4] fanotify24: Test open for write of executable files with pre-content watch

[Jan Kara]

On Mon 10-02-25 16:13:16, Amir Goldstein wrote:
> Watching pre-content events should allow opening an executable file for
> write and executing a file that is open for write.
> 
> We have an existing test where the exectable file is not watched by
> pre-content events.
> 
> We add a new test case, where the executable file is watched for
> FAN_PRE_ACCESS pre-content event and access is allowed.
> 
> In the former case (not watched), execution should fail with ETXTBSY and
> in the latter case (per-content watched) execution should succeed.
> 
> When allowing access events, we allow for multiple events, because
> read() may generate more than a single access event.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify24.c     | 43 ++++++++++++++++++-
>  1 file changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify24.c b/testcases/kernel/syscalls/fanotify/fanotify24.c
> index a7aa2e052..537773d52 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify24.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify24.c
> @@ -144,6 +144,15 @@ static struct tcase {
>  		{
>  		}
>  	},
> +	{
> +		"inode mark, FAN_PRE_ACCESS event allowed",
> +		INIT_FANOTIFY_MARK_TYPE(INODE),
> +		FAN_PRE_ACCESS, 1,
> +		{
> +			/* This allows multiple FAN_PRE_ACCESS events */
> +			{FAN_PRE_ACCESS, FAN_ALLOW},
> +		}
> +	},
>  };
>  
>  static int expected_errno(unsigned int response)
> @@ -206,6 +215,21 @@ static void generate_events(struct tcase *tc)
>  	exp_errno = expected_errno(event->response);
>  	event++;
>  
> +	/*
> +	 * If execve() is allowed by permission events, check if executing a
> +	 * file that open for write is allowed.
> +	 * HSM needs to be able to write to file during pre-content event, so it
> +	 * requires that a file being executed can be open for write, which also
> +	 * means that a file open for write can be executed.
> +	 * Therefore, ETXTBSY is to be expected when file is not being watched
> +	 * at all or being watched but not with pre-content events in mask.
> +	 */
> +	if (!exp_errno) {
> +		fd = SAFE_OPEN(FILE_EXEC_PATH, O_RDWR);
> +		if (!tc->event_count)
> +			exp_errno = ETXTBSY;
> +	}
> +
>  	exp_ret = exp_errno ? -1 : 0;
>  	errno = 0;
>  	if (execve(FILE_EXEC_PATH, argv, environ) != exp_ret || errno != exp_errno) {
> @@ -214,6 +238,9 @@ static void generate_events(struct tcase *tc)
>  	} else if (errno == exp_errno) {
>  		tst_res(TINFO, "execve() got errno %d as expected", errno);
>  	}
> +
> +	if (fd >= 0)
> +		SAFE_CLOSE(fd);
>  }
>  
>  static void child_handler(int tmp)
> @@ -309,8 +336,8 @@ static void test_fanotify(unsigned int n)
>  	/*
>  	 * Process events
>  	 *
> -	 * tc->count + 1 is to accommodate for checking the child process
> -	 * return value
> +	 * tc->count + 1 is to let read() wait for child process to exit
> +	 * and to accomodate for extra access events
>  	 */
>  	while (test_num < tc->event_count + 1 && fd_notify != -1) {
>  		struct fanotify_event_metadata *event;
> @@ -319,6 +346,7 @@ static void test_fanotify(unsigned int n)
>  			/* Get more events */
>  			ret = read(fd_notify, event_buf + len,
>  				   EVENT_BUF_LEN - len);
> +			/* Received SIGCHLD */
>  			if (fd_notify == -1)
>  				break;
>  			if (ret < 0) {
> @@ -329,6 +357,17 @@ static void test_fanotify(unsigned int n)
>  			len += ret;
>  		}
>  
> +		/*
> +		 * If we got an event after the last event and the last event was
> +		 * allowed then assume this is another event of the same type.
> +		 * This is to accomodate for the fact that a single read() may
> +		 * generate an unknown number of access permission events if they
> +		 * are allowed.
> +		 */
> +		if (test_num > 0 && test_num == tc->event_count &&
> +		    event_set[test_num-1].response == FAN_ALLOW)
> +			test_num--;
> +
>  		event = (struct fanotify_event_metadata *)&event_buf[i];
>  		/* Permission events cannot be merged, so the event mask
>  		 * reported should exactly match the event mask within the
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR