From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> stable inclusion from stable-v6.6.146 commit d8a1f7420d2d58fcbde479bd72d0900fb512cb2d category: bugfix bugzilla: NA CVE: NA Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=... -------------------------------- [ Upstream commit cd3f8467afd470ccab0de2fbc7c76664af4a0bac ] mm_access() can return NULL if the mm is not found, but this is handled the same as an error in all callers, with some translating this into an -ESRCH error. Only proc_mem_open() returns NULL if no mm is found, however in this case it is clearer and makes more sense to explicitly handle the error. Additionally we take the opportunity to refactor the function to eliminate unnecessary nesting. d8a1f7420d2d58fcbde479bd72d0900fb512cb2d Simplify things by simply returning -ESRCH if no mm is found - this both eliminates confusing use of the IS_ERR_OR_NULL() macro, and simplifies callers which would return -ESRCH by returning this error directly. [lorenzo.stoakes@oracle.com: prefer neater pointer error comparison] Link: https://lkml.kernel.org/r/2fae1834-749a-45e1-8594-5e5979cf7103@lucifer.local Link: https://lkml.kernel.org/r/20240924201023.193135-1-lorenzo.stoakes@oracle.com Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> Suggested-by: Arnd Bergmann <arnd@arndb.de> Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Tang Hui <tanghui20@huawei.com> --- fs/proc/base.c | 26 ++++++++++++++------------ kernel/fork.c | 5 +++-- mm/madvise.c | 4 ++-- mm/process_vm_access.c | 4 ++-- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index cac883cf29fa..7ee2285910bb 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -815,19 +815,21 @@ static const struct file_operations proc_single_file_operations = { struct mm_struct *proc_mem_open(struct inode *inode, unsigned int mode) { struct task_struct *task = get_proc_task(inode); - struct mm_struct *mm = ERR_PTR(-ESRCH); + struct mm_struct *mm; - if (task) { - mm = mm_access(task, mode | PTRACE_MODE_FSCREDS); - put_task_struct(task); + if (!task) + return ERR_PTR(-ESRCH); - if (!IS_ERR_OR_NULL(mm)) { - /* ensure this mm_struct can't be freed */ - mmgrab(mm); - /* but do not pin its memory */ - mmput(mm); - } - } + mm = mm_access(task, mode | PTRACE_MODE_FSCREDS); + put_task_struct(task); + + if (IS_ERR(mm)) + return mm == ERR_PTR(-ESRCH) ? NULL : mm; + + /* ensure this mm_struct can't be freed */ + mmgrab(mm); + /* but do not pin its memory */ + mmput(mm); return mm; } @@ -2205,7 +2207,7 @@ static int map_files_d_revalidate(struct dentry *dentry, unsigned int flags) goto out_notask; mm = mm_access(task, PTRACE_MODE_READ_FSCREDS); - if (IS_ERR_OR_NULL(mm)) + if (IS_ERR(mm)) goto out; if (!dname_to_vma_addr(dentry, &vm_start, &vm_end)) { diff --git a/kernel/fork.c b/kernel/fork.c index e92ddc0a0c66..7f188d166f35 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1676,8 +1676,9 @@ struct mm_struct *mm_access(struct task_struct *task, unsigned int mode) return ERR_PTR(err); mm = get_task_mm(task); - if (mm && mm != current->mm && - !ptrace_may_access(task, mode)) { + if (!mm) { + mm = ERR_PTR(-ESRCH); + } else if (mm != current->mm && !ptrace_may_access(task, mode)) { mmput(mm); mm = ERR_PTR(-EACCES); } diff --git a/mm/madvise.c b/mm/madvise.c index 4b7956dd6357..7fb856fc7eec 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -1846,8 +1846,8 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec, /* Require PTRACE_MODE_READ to avoid leaking ASLR metadata. */ mm = mm_access(task, PTRACE_MODE_READ_FSCREDS); - if (IS_ERR_OR_NULL(mm)) { - ret = IS_ERR(mm) ? PTR_ERR(mm) : -ESRCH; + if (IS_ERR(mm)) { + ret = PTR_ERR(mm); goto release_task; } diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c index 0523edab03a6..5be8e91aa872 100644 --- a/mm/process_vm_access.c +++ b/mm/process_vm_access.c @@ -200,8 +200,8 @@ static ssize_t process_vm_rw_core(pid_t pid, struct iov_iter *iter, } mm = mm_access(task, PTRACE_MODE_ATTACH_REALCREDS); - if (!mm || IS_ERR(mm)) { - rc = IS_ERR(mm) ? PTR_ERR(mm) : -ESRCH; + if (IS_ERR(mm)) { + rc = PTR_ERR(mm); /* * Explicitly map EACCES to EPERM as EPERM is a more * appropriate error code for process_vw_readv/writev -- 2.34.1