| Message ID | 20221202013404.163143-2-jeffxu@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kees Cook |
| Headers | show |
| Series | [v3] mm/memfd: add F_SEAL_EXEC | expand |
On Fri, Dec 02, 2022 at 01:33:59AM +0000, jeffxu@chromium.org wrote: > From: Daniel Verkamp <dverkamp@chromium.org> > > 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@chromium.org> > Signed-off-by: Jeff Xu <jeffxu@chromium.org> > Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> > --- > 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@chromium.org>
On Fri, Dec 02, 2022 at 01:33:59AM +0000, jeffxu@chromium.org wrote: > From: Daniel Verkamp <dverkamp@chromium.org> > > 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@chromium.org> > Signed-off-by: Jeff Xu <jeffxu@chromium.org> > Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Oh, one note on tag ordering here. Since you're sending it, I would expect this to read as: From: Daniel Verkamp <dverkamp@chromium.org> ... Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Co-developed-by: Jeff Xu <jeffxu@chromium.org> Signed-off-by: Jeff Xu <jeffxu@chromium.org>
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;