ACK: [Trusty SRU][PATCH 1/2] vfs: Commit to never having exectuables on proc and sysfs.
Colin Ian King
colin.king at canonical.com
Tue Sep 5 09:29:15 UTC 2017
On 04/09/17 18:54, Kleber Sacilotto de Souza wrote:
> From: "Eric W. Biederman" <ebiederm at xmission.com>
>
> CVE-2016-10044
>
> Today proc and sysfs do not contain any executable files. Several
> applications today mount proc or sysfs without noexec and nosuid and
> then depend on there being no exectuables files on proc or sysfs.
> Having any executable files show on proc or sysfs would cause
> a user space visible regression, and most likely security problems.
>
> Therefore commit to never allowing executables on proc and sysfs by
> adding a new flag to mark them as filesystems without executables and
> enforce that flag.
>
> Test the flag where MNT_NOEXEC is tested today, so that the only user
> visible effect will be that exectuables will be treated as if the
> execute bit is cleared.
>
> The filesystems proc and sysfs do not currently incoporate any
> executable files so this does not result in any user visible effects.
>
> This makes it unnecessary to vet changes to proc and sysfs tightly for
> adding exectuable files or changes to chattr that would modify
> existing files, as no matter what the individual file say they will
> not be treated as exectuable files by the vfs.
>
> Not having to vet changes to closely is important as without this we
> are only one proc_create call (or another goof up in the
> implementation of notify_change) from having problematic executables
> on proc. Those mistakes are all too easy to make and would create
> a situation where there are security issues or the assumptions of
> some program having to be broken (and cause userspace regressions).
>
> Signed-off-by: "Eric W. Biederman" <ebiederm at xmission.com>
> (backported from commit 90f8572b0f021fdd1baa68e00a8c30482ee9e5f4)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
> ---
> fs/exec.c | 10 ++++++++--
> fs/open.c | 2 +-
> fs/proc/root.c | 2 ++
> fs/sysfs/mount.c | 3 +++
> include/linux/fs.h | 2 ++
> kernel/sys.c | 3 +--
> mm/mmap.c | 4 ++--
> mm/nommu.c | 2 +-
> security/security.c | 2 +-
> 9 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index dd7ab64bd5a2..f96daeca68a9 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -107,6 +107,12 @@ bool path_nosuid(const struct path *path)
> }
> EXPORT_SYMBOL(path_nosuid);
>
> +bool path_noexec(const struct path *path)
> +{
> + return (path->mnt->mnt_flags & MNT_NOEXEC) ||
> + (path->mnt->mnt_sb->s_iflags & SB_I_NOEXEC);
> +}
> +
> /*
> * Note that a shared library must be both readable and executable due to
> * security reasons.
> @@ -140,7 +146,7 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
> goto exit;
>
> error = -EACCES;
> - if (file->f_path.mnt->mnt_flags & MNT_NOEXEC)
> + if (path_noexec(&file->f_path))
> goto exit;
>
> fsnotify_open(file);
> @@ -798,7 +804,7 @@ struct file *open_exec(const char *name)
> if (!S_ISREG(file_inode(file)->i_mode))
> goto exit;
>
> - if (file->f_path.mnt->mnt_flags & MNT_NOEXEC)
> + if (path_noexec(&file->f_path))
> goto exit;
>
> fsnotify_open(file);
> diff --git a/fs/open.c b/fs/open.c
> index 6dcf14157e27..d8aa5ebbbf84 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -344,7 +344,7 @@ retry:
> * with the "noexec" flag.
> */
> res = -EACCES;
> - if (path.mnt->mnt_flags & MNT_NOEXEC)
> + if (path_noexec(&path))
> goto out_path_release;
> }
>
> diff --git a/fs/proc/root.c b/fs/proc/root.c
> index c198775a5617..3ce05378b960 100644
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@ -139,6 +139,8 @@ static struct dentry *proc_mount(struct file_system_type *fs_type,
> }
>
> sb->s_flags |= MS_ACTIVE;
> + /* User space would break if executables appear on proc */
> + sb->s_iflags |= SB_I_NOEXEC;
> }
>
> return dget(sb->s_root);
> diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
> index f4d0799b5779..d705cabe9d6a 100644
> --- a/fs/sysfs/mount.c
> +++ b/fs/sysfs/mount.c
> @@ -139,6 +139,9 @@ static struct dentry *sysfs_mount(struct file_system_type *fs_type,
> }
> sb->s_flags |= MS_ACTIVE;
> }
> + if (sb->s_root)
> + /* Userspace would break if executables appear on sysfs */
> + sb->s_root->d_sb->s_iflags |= SB_I_NOEXEC;
>
> return dget(sb->s_root);
> }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 7e76dbc028d5..67a0bb3f3b19 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1221,6 +1221,7 @@ extern struct list_head super_blocks;
> extern spinlock_t sb_lock;
>
> /* sb->s_iflags */
> +#define SB_I_NOEXEC 0x00000002 /* Ignore executables on this fs */
> #define SB_I_NOSUID 0x00000004 /* Ignore suid on this fs */
>
> /* Possible states of 'frozen' field */
> @@ -2831,5 +2832,6 @@ static inline bool dir_relax(struct inode *inode)
> }
>
> extern bool path_nosuid(const struct path *path);
> +extern bool path_noexec(const struct path *path);
>
> #endif /* _LINUX_FS_H */
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 17b7417d0bf6..3b663a209691 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -1648,8 +1648,7 @@ static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
> * overall picture.
> */
> err = -EACCES;
> - if (!S_ISREG(inode->i_mode) ||
> - exe.file->f_path.mnt->mnt_flags & MNT_NOEXEC)
> + if (!S_ISREG(inode->i_mode) || path_noexec(&exe.file->f_path))
> goto exit;
>
> err = inode_permission(inode, MAY_EXEC);
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 72f35bce0f16..8cbdd057bf83 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1261,7 +1261,7 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
> * mounted, in which case we dont add PROT_EXEC.)
> */
> if ((prot & PROT_READ) && (current->personality & READ_IMPLIES_EXEC))
> - if (!(file && (file->f_path.mnt->mnt_flags & MNT_NOEXEC)))
> + if (!(file && path_noexec(&file->f_path)))
> prot |= PROT_EXEC;
>
> if (!len)
> @@ -1341,7 +1341,7 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
> case MAP_PRIVATE:
> if (!(file->f_mode & FMODE_READ))
> return -EACCES;
> - if (file->f_path.mnt->mnt_flags & MNT_NOEXEC) {
> + if (path_noexec(&file->f_path)) {
> if (vm_flags & VM_EXEC)
> return -EPERM;
> vm_flags &= ~VM_MAYEXEC;
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 4efb31662bd1..e3053616ffad 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -1031,7 +1031,7 @@ static int validate_mmap_request(struct file *file,
>
> /* handle executable mappings and implied executable
> * mappings */
> - if (file->f_path.mnt->mnt_flags & MNT_NOEXEC) {
> + if (path_noexec(&file->f_path)) {
> if (prot & PROT_EXEC)
> return -EPERM;
> }
> diff --git a/security/security.c b/security/security.c
> index ae6eba6b010d..79d239fdba70 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -719,7 +719,7 @@ static inline unsigned long mmap_prot(struct file *file, unsigned long prot)
> * ditto if it's not on noexec mount, except that on !MMU we need
> * BDI_CAP_EXEC_MMAP (== VM_MAYEXEC) in this case
> */
> - if (!(file->f_path.mnt->mnt_flags & MNT_NOEXEC)) {
> + if (!path_noexec(&file->f_path)) {
> #ifndef CONFIG_MMU
> unsigned long caps = 0;
> struct address_space *mapping = file->f_mapping;
>
Looks like a sane backport to me.
Acked-by: Colin Ian King <colin.king at canonical.com>
More information about the kernel-team
mailing list