From f6caf248723381e6cc48f9fd4132407ac3fba43f Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 16 Sep 2022 17:11:18 -0700 Subject: [PATCH 1/2] exec: Add do_close_execat() helper ANBZ: #33963 commit bdd8f62431ebcf15902a5fce3336388e436405c6 upstream. Consolidate the calls to allow_write_access()/fput() into a single place, since we repeat this code pattern. Add comments around the callers for the details on it. Link: https://lore.kernel.org/r/202209161637.9EDAF6B18@keescook Signed-off-by: Kees Cook Conflicts: fs/exec.c [ Gao Xiang: Note that commit 978ffcbf00d8 ("execve: open the executable file before doing anything else") isn't backported since it will make the dependency more complex. This commit just introduces a helper so it's more trivial to resolve the conflict directly instead. ] Signed-off-by: Gao Xiang --- fs/exec.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 31f3aafc18ca..e5ee7e60d22b 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -903,6 +903,10 @@ EXPORT_SYMBOL(transfer_args_to_stack); #endif /* CONFIG_MMU */ +/* + * On success, caller must call do_close_execat() on the returned + * struct file to close it. + */ static struct file *do_open_execat(int fd, struct filename *name, int flags) { struct file *file; @@ -946,6 +950,17 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) return ERR_PTR(err); } +/** + * open_exec - Open a path name for execution + * + * @name: path name to open with the intent of executing it. + * + * Returns ERR_PTR on failure or allocated struct file on success. + * + * As this is a wrapper for the internal do_open_execat(), callers + * must call allow_write_access() before fput() on release. Also see + * do_close_execat(). + */ struct file *open_exec(const char *name) { struct filename *filename = getname_kernel(name); @@ -1509,6 +1524,15 @@ static int prepare_bprm_creds(struct linux_binprm *bprm) return -ENOMEM; } +/* Matches do_open_execat() */ +static void do_close_execat(struct file *file) +{ + if (!file) + return; + allow_write_access(file); + fput(file); +} + static void free_bprm(struct linux_binprm *bprm) { if (bprm->mm) { @@ -1522,10 +1546,7 @@ static void free_bprm(struct linux_binprm *bprm) mutex_unlock(¤t->signal->cred_guard_mutex); abort_creds(bprm->cred); } - if (bprm->file) { - allow_write_access(bprm->file); - fput(bprm->file); - } + do_close_execat(bprm->file); if (bprm->executable) fput(bprm->executable); /* If a binfmt changed the interp, free it. */ -- Gitee From 2e3924c68232d43e842a0dcb4323af028395409e Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Thu, 28 Nov 2024 15:25:32 +0100 Subject: [PATCH 2/2] fs: don't block write during exec on pre-content watched files ANBZ: #33963 commit 0357ef03c94ef835bd44a0658b8edb672a9dbf51 upstream. Commit 2a010c412853 ("fs: don't block i_writecount during exec") removed the legacy behavior of getting ETXTBSY on attempt to open and executable file for write while it is being executed. This commit was reverted because an application that depends on this legacy behavior was broken by the change. We need to allow HSM writing into executable files while executed to fill their content on-the-fly. To that end, disable the ETXTBSY legacy behavior for files that are watched by pre-content events. This change is not expected to cause regressions with existing systems which do not have any pre-content event listeners. Signed-off-by: Amir Goldstein Acked-by: Christian Brauner Signed-off-by: Jan Kara Link: https://patch.msgid.link/20241128142532.465176-1-amir73il@gmail.com Signed-off-by: Gao Xiang --- fs/binfmt_elf.c | 4 ++-- fs/binfmt_elf_fdpic.c | 4 ++-- fs/exec.c | 8 ++++---- include/linux/fs.h | 22 ++++++++++++++++++++++ kernel/fork.c | 12 ++++++------ 5 files changed, 36 insertions(+), 14 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 02bef57a880c..261074367bdf 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -1291,7 +1291,7 @@ static int load_elf_binary(struct linux_binprm *bprm) } reloc_func_desc = interp_load_addr; - allow_write_access(interpreter); + exe_file_allow_write_access(interpreter); fput(interpreter); kfree(interp_elf_ex); @@ -1403,7 +1403,7 @@ static int load_elf_binary(struct linux_binprm *bprm) kfree(interp_elf_ex); kfree(interp_elf_phdata); out_free_file: - allow_write_access(interpreter); + exe_file_allow_write_access(interpreter); if (interpreter) fput(interpreter); out_free_ph: diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c index 96a8b13b57d9..13360ec5da92 100644 --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -394,7 +394,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm) goto error; } - allow_write_access(interpreter); + exe_file_allow_write_access(interpreter); fput(interpreter); interpreter = NULL; } @@ -467,7 +467,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm) error: if (interpreter) { - allow_write_access(interpreter); + exe_file_allow_write_access(interpreter); fput(interpreter); } kfree(interpreter_name); diff --git a/fs/exec.c b/fs/exec.c index e5ee7e60d22b..4c4caaf123b9 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -939,7 +939,7 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) path_noexec(&file->f_path)) goto exit; - err = deny_write_access(file); + err = exe_file_deny_write_access(file); if (err) goto exit; @@ -958,7 +958,7 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) * Returns ERR_PTR on failure or allocated struct file on success. * * As this is a wrapper for the internal do_open_execat(), callers - * must call allow_write_access() before fput() on release. Also see + * must call exe_file_allow_write_access() before fput() on release. Also see * do_close_execat(). */ struct file *open_exec(const char *name) @@ -1529,7 +1529,7 @@ static void do_close_execat(struct file *file) { if (!file) return; - allow_write_access(file); + exe_file_allow_write_access(file); fput(file); } @@ -1847,7 +1847,7 @@ static int exec_binprm(struct linux_binprm *bprm) bprm->file = bprm->interpreter; bprm->interpreter = NULL; - allow_write_access(exec); + exe_file_allow_write_access(exec); if (unlikely(bprm->have_execfd)) { if (bprm->executable) { fput(exec); diff --git a/include/linux/fs.h b/include/linux/fs.h index 0e296544eb59..20bf3da75198 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3015,6 +3015,28 @@ static inline void allow_write_access(struct file *file) if (file) atomic_inc(&file_inode(file)->i_writecount); } + +/* + * Do not prevent write to executable file when watched by pre-content events. + * + * Note that FMODE_FSNOTIFY_HSM mode is set depending on pre-content watches at + * the time of file open and remains constant for entire lifetime of the file, + * so if pre-content watches are added post execution or removed before the end + * of the execution, it will not cause i_writecount reference leak. + */ +static inline int exe_file_deny_write_access(struct file *exe_file) +{ + if (unlikely(FMODE_FSNOTIFY_HSM(exe_file->f_mode))) + return 0; + return deny_write_access(exe_file); +} +static inline void exe_file_allow_write_access(struct file *exe_file) +{ + if (unlikely(!exe_file || FMODE_FSNOTIFY_HSM(exe_file->f_mode))) + return; + allow_write_access(exe_file); +} + static inline bool inode_is_open_for_write(const struct inode *inode) { return atomic_read(&inode->i_writecount) > 0; diff --git a/kernel/fork.c b/kernel/fork.c index 65db0a18041c..57b055ea1e02 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -648,8 +648,8 @@ static void dup_mm_exe_file(struct mm_struct *mm, struct mm_struct *oldmm) * We depend on the oldmm having properly denied write access to the * exe_file already. */ - if (exe_file && deny_write_access(exe_file)) - pr_warn_once("deny_write_access() failed in %s\n", __func__); + if (exe_file && exe_file_deny_write_access(exe_file)) + pr_warn_once("exe_file_deny_write_access() failed in %s\n", __func__); } #ifdef CONFIG_MMU @@ -1478,13 +1478,13 @@ int set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file) * We expect the caller (i.e., sys_execve) to already denied * write access, so this is unlikely to fail. */ - if (unlikely(deny_write_access(new_exe_file))) + if (unlikely(exe_file_deny_write_access(new_exe_file))) return -EACCES; get_file(new_exe_file); } rcu_assign_pointer(mm->exe_file, new_exe_file); if (old_exe_file) { - allow_write_access(old_exe_file); + exe_file_allow_write_access(old_exe_file); fput(old_exe_file); } return 0; @@ -1523,7 +1523,7 @@ int replace_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file) return ret; } - ret = deny_write_access(new_exe_file); + ret = exe_file_deny_write_access(new_exe_file); if (ret) return -EACCES; get_file(new_exe_file); @@ -1535,7 +1535,7 @@ int replace_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file) mmap_write_unlock(mm); if (old_exe_file) { - allow_write_access(old_exe_file); + exe_file_allow_write_access(old_exe_file); fput(old_exe_file); } return 0; -- Gitee