Re: + smaps-add-clear_refs-file-to-clear-reference.patch added to -mm tree

Re: + smaps-add-clear_refs-file-to-clear-reference.patch added to -mm tree

[Oleg Nesterov]

David Rientjes wrote:
>
> +static ssize_t clear_refs_write(struct file *file, const char __user *buf,
> +				size_t count, loff_t *ppos)
> +{
> ...
> +	task = get_proc_task(file->f_path.dentry->d_inode);
> +	if (!task)
> +		return -ESRCH;
> +	clear_refs_smap(task->mm->mmap);

task->mm may be NULL and not stable, this needs get_task_mm() (may fail).

Don't we also need ->mmap_sem to iterate vmas?

Oleg.

Re: + smaps-add-clear_refs-file-to-clear-reference.patch added to -mm tree

[Andrew Morton]

On Sat, 10 Feb 2007 03:39:58 +0300
Oleg Nesterov <oleg@tv-sign.ru> wrote:

> David Rientjes wrote:
> >
> > +static ssize_t clear_refs_write(struct file *file, const char __user *buf,
> > +				size_t count, loff_t *ppos)
> > +{
> > ...
> > +	task = get_proc_task(file->f_path.dentry->d_inode);
> > +	if (!task)
> > +		return -ESRCH;
> > +	clear_refs_smap(task->mm->mmap);
> 
> task->mm may be NULL and not stable, this needs get_task_mm() (may fail).

yup.

> Don't we also need ->mmap_sem to iterate vmas?

and yup.

Like this?

--- a/fs/proc/base.c~smaps-add-clear_refs-file-to-clear-reference-fix
+++ a/fs/proc/base.c
@@ -720,6 +720,7 @@ static ssize_t clear_refs_write(struct f
 {
 	struct task_struct *task;
 	char buffer[PROC_NUMBUF], *end;
+	struct mm_struct *mm;
 
 	memset(buffer, 0, sizeof(buffer));
 	if (count > sizeof(buffer) - 1)
@@ -733,7 +734,11 @@ static ssize_t clear_refs_write(struct f
 	task = get_proc_task(file->f_path.dentry->d_inode);
 	if (!task)
 		return -ESRCH;
-	clear_refs_smap(task->mm->mmap);
+	mm = get_task_mm(task);
+	if (mm) {
+		clear_refs_smap(mm);
+		mmput(mm);
+	}
 	put_task_struct(task);
 	if (end - buffer == 0)
 		return -EIO;
diff -puN fs/proc/task_mmu.c~smaps-add-clear_refs-file-to-clear-reference-fix fs/proc/task_mmu.c
--- a/fs/proc/task_mmu.c~smaps-add-clear_refs-file-to-clear-reference-fix
+++ a/fs/proc/task_mmu.c
@@ -350,11 +350,15 @@ static int show_smap(struct seq_file *m,
 	return show_map_internal(m, v, &mss);
 }
 
-void clear_refs_smap(struct vm_area_struct *vma)
+void clear_refs_smap(struct mm_struct *mm)
 {
-	for (; vma; vma = vma->vm_next)
+	struct vm_area_struct *vma;
+
+	down_read(&mm->mmap_sem);
+	for (vma = mm->mmap; vma; vma = vma->vm_next)
 		if (vma->vm_mm && !is_vm_hugetlb_page(vma))
 			for_each_pmd(vma, clear_refs_one_pmd, NULL);
+	up_read(&mm->mmap_sem);
 }
 
 static void *m_start(struct seq_file *m, loff_t *pos)
diff -puN include/linux/proc_fs.h~smaps-add-clear_refs-file-to-clear-reference-fix include/linux/proc_fs.h
--- a/include/linux/proc_fs.h~smaps-add-clear_refs-file-to-clear-reference-fix
+++ a/include/linux/proc_fs.h
@@ -104,7 +104,7 @@ int proc_pid_readdir(struct file * filp,
 unsigned long task_vsize(struct mm_struct *);
 int task_statm(struct mm_struct *, int *, int *, int *, int *);
 char *task_mem(struct mm_struct *, char *);
-void clear_refs_smap(struct vm_area_struct *);
+void clear_refs_smap(struct mm_struct *mm);
 
 extern struct proc_dir_entry *create_proc_entry(const char *name, mode_t mode,
 						struct proc_dir_entry *parent);
_

Re: + smaps-add-clear_refs-file-to-clear-reference.patch added to -mm tree

[David Rientjes]

Do not clear references when the task_struct's mm is NULL by using 
/proc/pid/clear_refs.

Also, use mmap_sem since the mm_struct's VMA's are being iterated in 
fs/proc/task_mmu.c.

Reported by Oleg Nesterov <oleg@tv-sign.ru>.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 fs/proc/base.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -719,6 +719,7 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
 				size_t count, loff_t *ppos)
 {
 	struct task_struct *task;
+	struct mm_struct *mm;
 	char buffer[PROC_NUMBUF], *end;
 
 	memset(buffer, 0, sizeof(buffer));
@@ -733,7 +734,13 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
 	task = get_proc_task(file->f_path.dentry->d_inode);
 	if (!task)
 		return -ESRCH;
-	clear_refs_smap(task->mm->mmap);
+	mm = get_task_mm(task);
+	if (mm) {
+		down_read(&mm->mmap_sem);
+		clear_refs_smap(mm->mmap);
+		up_read(&mm->mmap_sem);
+		mmput(mm);
+	}
 	put_task_struct(task);
 	if (end - buffer == 0)
 		return -EIO;