On Fri, Dec 02, 2022 at 01:33:59AM +0000, jeffxu@xxxxxxxxxxxx wrote:
> From: Daniel Verkamp <dverkamp@xxxxxxxxxxxx>
>
> The new F_SEAL_EXEC flag will prevent modification of the exec bits:
> written as traditional octal mask, 0111, or as named flags, S_IXUSR |
> S_IXGRP | S_IXOTH. Any chmod(2) or similar call that attempts to modify
> any of these bits after the seal is applied will fail with errno EPERM.
>
> This will preserve the execute bits as they are at the time of sealing,
> so the memfd will become either permanently executable or permanently
> un-executable.
>
> Co-developed-by: Jeff Xu <jeffxu@xxxxxxxxxxxx>
> Signed-off-by: Jeff Xu <jeffxu@xxxxxxxxxxxx>
> Signed-off-by: Daniel Verkamp <dverkamp@xxxxxxxxxxxx>
> ---
> include/uapi/linux/fcntl.h | 1 +
> mm/memfd.c | 2 ++
> mm/shmem.c | 6 ++++++
> 3 files changed, 9 insertions(+)
>
> diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> index 2f86b2ad6d7e..e8c07da58c9f 100644
> --- a/include/uapi/linux/fcntl.h
> +++ b/include/uapi/linux/fcntl.h
> @@ -43,6 +43,7 @@
> #define F_SEAL_GROW 0x0004 /* prevent file from growing */
> #define F_SEAL_WRITE 0x0008 /* prevent writes */
> #define F_SEAL_FUTURE_WRITE 0x0010 /* prevent future writes while mapped */
> +#define F_SEAL_EXEC 0x0020 /* prevent chmod modifying exec bits */
> /* (1U << 31) is reserved for signed error codes */
>
> /*
> diff --git a/mm/memfd.c b/mm/memfd.c
> index 08f5f8304746..4ebeab94aa74 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -147,6 +147,7 @@ static unsigned int *memfd_file_seals_ptr(struct file *file)
> }
>
> #define F_ALL_SEALS (F_SEAL_SEAL | \
> + F_SEAL_EXEC | \
> F_SEAL_SHRINK | \
> F_SEAL_GROW | \
> F_SEAL_WRITE | \
> @@ -175,6 +176,7 @@ static int memfd_add_seals(struct file *file, unsigned int seals)
> * SEAL_SHRINK: Prevent the file from shrinking
> * SEAL_GROW: Prevent the file from growing
> * SEAL_WRITE: Prevent write access to the file
> + * SEAL_EXEC: Prevent modification of the exec bits in the file mode
> *
> * As we don't require any trust relationship between two parties, we
> * must prevent seals from being removed. Therefore, sealing a file
> diff --git a/mm/shmem.c b/mm/shmem.c
> index c1d8b8a1aa3b..e18a9cf9d937 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1085,6 +1085,12 @@ static int shmem_setattr(struct user_namespace *mnt_userns,
> if (error)
> return error;
>
> + if ((info->seals & F_SEAL_EXEC) && (attr->ia_valid & ATTR_MODE)) {
> + if ((inode->i_mode ^ attr->ia_mode) & 0111) {
> + return -EPERM;
> + }
> + }
> +
> if (S_ISREG(inode->i_mode) && (attr->ia_valid & ATTR_SIZE)) {
> loff_t oldsize = inode->i_size;
> loff_t newsize = attr->ia_size;
> --
> 2.39.0.rc0.267.gcb52ba06e7-goog
>
This looks sensible to me!
Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
--
Kees Cook
