linux-kernel - [PATCH V3] proc: Fix the issue of proc_mem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250331091635.36547-1-superman.xpt@gmail.com>
Date: Mon, 31 Mar 2025 02:16:35 -0700
From: Penglei Jiang <superman.xpt@...il.com>
To: akpm@...ux-foundation.org
Cc: adrian.ratiu@...labora.com,
	brauner@...nel.org,
	felix.moessbauer@...mens.com,
	jlayton@...nel.org,
	linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	lorenzo.stoakes@...cle.com,
	mjguzik@...il.com,
	superman.xpt@...il.com,
	syzbot+02e64be5307d72e9c309@...kaller.appspotmail.com,
	syzbot+f9238a0a31f9b5603fef@...kaller.appspotmail.com,
	tglx@...utronix.de,
	viro@...iv.linux.org.uk,
	xu.xin16@....com.cn
Subject: [PATCH V3] proc: Fix the issue of proc_mem_open returning NULL

On Thu, 27 Mar 2025 12:24:45 -0700 Andrew Morton <akpm@...ux-foundation.org> wrote:

> On Mon, 24 Mar 2025 21:14:48 -0700 Penglei Jiang <superman.xpt@...il.com> wrote:
>
> > > >  if (IS_ERR(mm))
> > > > -return mm == ERR_PTR(-ESRCH) ? NULL : mm;
> > > > +return mm;
> > > >
> > > >  /* ensure this mm_struct can't be freed */
> > > >  mmgrab(mm);
> > > > --
> > > > 2.17.1
> > > >
> >
> > Mateusz Guzik provides valuable suggestions.
> >
> > Complete the missing NULL checks.
>
> proc_mem_open() can return errno, NULL or mm_struct*.  It isn't obvious
> why.
>
> While you're in there can you please add documentation to
> proc_mem_open() which explains its return values?

I apologize for the delayed response.

Add documentation comments to proc_mem_open() and add NULL checks in
several call sites.

Signed-off-by: Penglei Jiang <superman.xpt@...il.com>
---
 fs/proc/base.c       | 12 +++++++++---
 fs/proc/task_mmu.c   | 12 ++++++------
 fs/proc/task_nommu.c |  4 ++--
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 5538c4aee8fa..cbe4e7d557e1 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -827,7 +827,13 @@ static const struct file_operations proc_single_file_operations = {
 	.release	= single_release,
 };
 
-
+/*
+ * proc_mem_open() can return errno, NULL or mm_struct*.
+ *
+ *   - Returns NULL if the task has no mm (task->flags & PF_KTHREAD)
+ *   - Returns mm_struct* on success
+ *   - Returns error code on failure
+ */
 struct mm_struct *proc_mem_open(struct inode *inode, unsigned int mode)
 {
 	struct task_struct *task = get_proc_task(inode);
@@ -854,8 +860,8 @@ static int __mem_open(struct inode *inode, struct file *file, unsigned int mode)
 {
 	struct mm_struct *mm = proc_mem_open(inode, mode);
 
-	if (IS_ERR(mm))
-		return PTR_ERR(mm);
+	if (IS_ERR_OR_NULL(mm))
+		return mm ? PTR_ERR(mm) : -ESRCH;
 
 	file->private_data = mm;
 	return 0;
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index f02cd362309a..14d1d8d3e432 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -212,8 +212,8 @@ static int proc_maps_open(struct inode *inode, struct file *file,
 
 	priv->inode = inode;
 	priv->mm = proc_mem_open(inode, PTRACE_MODE_READ);
-	if (IS_ERR(priv->mm)) {
-		int err = PTR_ERR(priv->mm);
+	if (IS_ERR_OR_NULL(priv->mm)) {
+		int err = priv->mm ? PTR_ERR(priv->mm) : -ESRCH;
 
 		seq_release_private(inode, file);
 		return err;
@@ -1312,8 +1312,8 @@ static int smaps_rollup_open(struct inode *inode, struct file *file)
 
 	priv->inode = inode;
 	priv->mm = proc_mem_open(inode, PTRACE_MODE_READ);
-	if (IS_ERR(priv->mm)) {
-		ret = PTR_ERR(priv->mm);
+	if (IS_ERR_OR_NULL(priv->mm)) {
+		ret = priv->mm ? PTR_ERR(priv->mm) : -ESRCH;
 
 		single_release(inode, file);
 		goto out_free;
@@ -2045,8 +2045,8 @@ static int pagemap_open(struct inode *inode, struct file *file)
 	struct mm_struct *mm;
 
 	mm = proc_mem_open(inode, PTRACE_MODE_READ);
-	if (IS_ERR(mm))
-		return PTR_ERR(mm);
+	if (IS_ERR_OR_NULL(mm))
+		return mm ? PTR_ERR(mm) : -ESRCH;
 	file->private_data = mm;
 	return 0;
 }
diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index bce674533000..59bfd61d653a 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -260,8 +260,8 @@ static int maps_open(struct inode *inode, struct file *file,
 
 	priv->inode = inode;
 	priv->mm = proc_mem_open(inode, PTRACE_MODE_READ);
-	if (IS_ERR(priv->mm)) {
-		int err = PTR_ERR(priv->mm);
+	if (IS_ERR_OR_NULL(priv->mm)) {
+		int err = priv->mm ? PTR_ERR(priv->mm) : -ESRCH;
 
 		seq_release_private(inode, file);
 		return err;
-- 
2.17.1