[RFC PATCH 5/9] Define user structure for events and responses.

[Mickaël Salaün]

On Tue, Mar 04, 2025 at 01:13:01AM +0000, Tingmao Wang wrote:
> The two structures are designed to be passed via read and write
> to the supervisor-fd.  Compile time check for no holes are added
> to build_check_abi.
> 
> The event structure will be a dynamically sized structure with
> possibly a NULL-terminating filename at the end.  This is so that
> we can pass a raw filename to the supervisor for file creation
> requests, without having the trouble of not being able to open a
> fd to a file that has not been created.
> 
> NOTE: despite this patch having a new uapi, I'm still very open to e.g.
> re-using fanotify stuff instead (if that makes sense in the end). This is
> just a PoC.
> 
> Signed-off-by: Tingmao Wang <m at maowtm.org>
> ---
>  include/uapi/linux/landlock.h | 107 ++++++++++++++++++++++++++++++++++
>  security/landlock/syscalls.c  |  28 +++++++++
>  2 files changed, 135 insertions(+)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 7bc1eb4859fb..b5645fdd998d 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -318,4 +318,111 @@ struct landlock_net_port_attr {
>  #define LANDLOCK_SCOPE_SIGNAL		                (1ULL << 1)
>  /* clang-format on*/
>  
> +/**
> + * DOC: supervisor
> + *
> + * Supervise mode
> + * ~~~~~~~~~~~~~~
> + *
> + * TODO
> + */
> +
> +typedef __u16 landlock_supervise_event_type_t;
> +/* clang-format off */
> +#define LANDLOCK_SUPERVISE_EVENT_TYPE_FS_ACCESS         1
> +#define LANDLOCK_SUPERVISE_EVENT_TYPE_NET_ACCESS        2
> +/* clang-format on */
> +
> +struct landlock_supervise_event_hdr {
> +	/**
> +	 * @type: Type of the event.
> +	 */
> +	landlock_supervise_event_type_t type;
> +	/**
> +	 * @length: Length of the entire struct
> +	 * landlock_supervise_event including this header.
> +	 */
> +	__u16 length;
> +	/**
> +	 * @cookie: Opaque identifier to be included in the response.
> +	 */
> +	__u32 cookie;

I guess we could use a __u64 index counter per layer instead.  That
would also help to order requests if they are treated by different
supervisor threads.

> +};
> +
> +struct landlock_supervise_event {
> +	struct landlock_supervise_event_hdr hdr;
> +	__u64 access_request;
> +	__kernel_pid_t accessor;
> +	union {
> +		struct {
> +			/**
> +			 * @fd1: An open file descriptor for the file (open,
> +			 * delete, execute, link, readdir, rename, truncate),
> +			 * or the parent directory (for create operations
> +			 * targeting its child) being accessed.  Must be
> +			 * closed by the reader.
> +			 *
> +			 * If this points to a parent directory, @destname
> +			 * will contain the target filename. If @destname is
> +			 * empty, this points to the target file.
> +			 */
> +			int fd1;
> +			/**
> +			 * @fd2: For link or rename requests, a second file
> +			 * descriptor for the target parent directory.  Must
> +			 * be closed by the reader.  @destname contains the
> +			 * destination filename.  This field is -1 if not
> +			 * used.
> +			 */
> +			int fd2;

Can we just use one FD but identify the requested access instead and
send one event for each, like for the audit patch series?

> +			/**
> +			 * @destname: A filename for a file creation target.
> +			 *
> +			 * If either of fd1 or fd2 points to a parent
> +			 * directory rather than the target file, this is the
> +			 * NULL-terminated name of the file that will be
> +			 * newly created.
> +			 *
> +			 * Counting the NULL terminator, this field will
> +			 * contain one or more NULL padding at the end so
> +			 * that the length of the whole struct
> +			 * landlock_supervise_event is a multiple of 8 bytes.
> +			 *
> +			 * This is a variable length member, and the length
> +			 * including the terminating NULL(s) can be derived
> +			 * from hdr.length - offsetof(struct
> +			 * landlock_supervise_event, destname).
> +			 */
> +			char destname[];

I'd prefer to avoid sending file names for now.  I don't think it's
necessary, and that could encourage supervisors to filter access
according to names.

> +		};
> +		struct {
> +			__u16 port;
> +		};
> +	};
> +};
> +

[...]