| Message ID | 20170904175434.7071-2-kleber.souza@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Trusty,SRU,1/2] vfs: Commit to never having exectuables on proc and sysfs. | expand |
On 04/09/17 18:54, Kleber Sacilotto de Souza wrote: > From: "Eric W. Biederman" <ebiederm@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@xmission.com> > (backported from commit 90f8572b0f021fdd1baa68e00a8c30482ee9e5f4) > Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@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@canonical.com>
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;