Re: [PATCH] fix NULL pointer dereference in __vm_enough_memory()

Previous thread: Re: [PATCH] pata_artop: fix UDMA5 for AEC6280[R] and UDMA6 for AEC6880[R] by Mikael Pettersson on Sunday, August 12, 2007 - 4:53 am. (2 messages)

Next thread: Improving read/write/close system call reliability when used with pthreads by Fredrik Noring on Sunday, August 12, 2007 - 6:03 am. (9 messages)
From: WU Fengguang
Date: Sunday, August 12, 2007 - 5:27 am

Hi Balbir,

[add CC to LKML]



No I changed it several weeks ago to stop my desktop from freezing.
So yes, the bug may have been there for a while.

-

From: Alan Cox
Date: Sunday, August 12, 2007 - 6:19 am

The bug is the new exec with lots of arguments code. It tries to insert a
vm struct without having a valid current->mm. That isn't permitted and
never had been (which is also why it broke the sparc mmu code etc).

You'll need to change the kernel security interface a little to make this
fly - I think the following should do it.

	- make __vm_enough_memory take a struct mm pointer and use it
	- make security_ops pass the extra current->mm
	- add a vm_enough_memory_mm security op
	- use security_vm_enough_memory_mm(mm, ...) in __insert_vm_struct

I'll knock up a quick patch and see what is needed (someone else can do
the selinux changes)
-

From: WU Fengguang
Date: Sunday, August 12, 2007 - 7:09 am

Thank you!

Count me for one - but CC SELinux maintainers first :)

-

From: Alan Cox
Date: Sunday, August 12, 2007 - 8:17 am

Try this (it compiles but isnt tested). Its a weekend here, the sun is
shining, the beach is a short walk, and I have more interesting things to
do right now 8)


diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc1-mm1/include/linux/mm.h linux-2.6.23rc1-mm1/include/linux/mm.h
--- linux.vanilla-2.6.23rc1-mm1/include/linux/mm.h	2007-07-26 15:02:58.000000000 +0100
+++ linux-2.6.23rc1-mm1/include/linux/mm.h	2007-08-12 13:54:24.614647536 +0100
@@ -1079,7 +1079,7 @@
 }
 
 /* mmap.c */
-extern int __vm_enough_memory(long pages, int cap_sys_admin);
+extern int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin);
 extern void vma_adjust(struct vm_area_struct *vma, unsigned long start,
 	unsigned long end, pgoff_t pgoff, struct vm_area_struct *insert);
 extern struct vm_area_struct *vma_merge(struct mm_struct *,
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc1-mm1/include/linux/security.h linux-2.6.23rc1-mm1/include/linux/security.h
--- linux.vanilla-2.6.23rc1-mm1/include/linux/security.h	2007-07-26 15:02:58.000000000 +0100
+++ linux-2.6.23rc1-mm1/include/linux/security.h	2007-08-12 14:13:10.383504656 +0100
@@ -58,7 +58,7 @@
 extern int cap_task_setioprio (struct task_struct *p, int ioprio);
 extern int cap_task_setnice (struct task_struct *p, int nice);
 extern int cap_syslog (int type);
-extern int cap_vm_enough_memory (long pages);
+extern int cap_vm_enough_memory (struct mm_struct *mm, long pages);
 
 struct msghdr;
 struct sk_buff;
@@ -1129,6 +1129,7 @@
  *	Return 0 if permission is granted.
  * @vm_enough_memory:
  *	Check permissions for allocating a new virtual mapping.
+ *	@mm contains the mm struct it is being added to.
  *      @pages contains the number of pages.
  *	Return 0 if permission is granted.
  *
@@ -1173,7 +1174,7 @@
 	int (*quota_on) (struct dentry * dentry);
 	int (*syslog) (int type);
 	int (*settime) (struct timespec *ts, struct timezone *tz);
-	int ...
From: Cyrill Gorcunov
Date: Sunday, August 12, 2007 - 9:21 am

[Alan Cox - Sun, Aug 12, 2007 at 04:17:44PM +0100]
| Try this (it compiles but isnt tested). Its a weekend here, the sun is
| shining, the beach is a short walk, and I have more interesting things to
| do right now 8)
| 
| 
| diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc1-mm1/include/linux/mm.h linux-2.6.23rc1-mm1/include/linux/mm.h
| --- linux.vanilla-2.6.23rc1-mm1/include/linux/mm.h	2007-07-26 15:02:58.000000000 +0100
| +++ linux-2.6.23rc1-mm1/include/linux/mm.h	2007-08-12 13:54:24.614647536 +0100
| @@ -1079,7 +1079,7 @@
|  }
|  
|  /* mmap.c */
| -extern int __vm_enough_memory(long pages, int cap_sys_admin);
| +extern int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin);
|  extern void vma_adjust(struct vm_area_struct *vma, unsigned long start,
|  	unsigned long end, pgoff_t pgoff, struct vm_area_struct *insert);
|  extern struct vm_area_struct *vma_merge(struct mm_struct *,
| diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc1-mm1/include/linux/security.h linux-2.6.23rc1-mm1/include/linux/security.h
| --- linux.vanilla-2.6.23rc1-mm1/include/linux/security.h	2007-07-26 15:02:58.000000000 +0100
| +++ linux-2.6.23rc1-mm1/include/linux/security.h	2007-08-12 14:13:10.383504656 +0100
| @@ -58,7 +58,7 @@
|  extern int cap_task_setioprio (struct task_struct *p, int ioprio);
|  extern int cap_task_setnice (struct task_struct *p, int nice);
|  extern int cap_syslog (int type);
| -extern int cap_vm_enough_memory (long pages);
| +extern int cap_vm_enough_memory (struct mm_struct *mm, long pages);
|  
|  struct msghdr;
|  struct sk_buff;
| @@ -1129,6 +1129,7 @@
|   *	Return 0 if permission is granted.
|   * @vm_enough_memory:
|   *	Check permissions for allocating a new virtual mapping.
| + *	@mm contains the mm struct it is being added to.
|   *      @pages contains the number of pages.
|   *	Return 0 if permission is granted.
|   *
| @@ -1173,7 +1174,7 @@
|  	int (*quota_on) ...
From: WU Fengguang
Date: Sunday, August 12, 2007 - 5:23 pm

Yeah, Alan adds mm to the interfaces and leaves us the question of
"what mm to pass in when current->mm == NULL?" ;)

-

From: Rene Herman
Date: Sunday, August 12, 2007 - 5:14 pm

Oh come on, you have a beard. You can't go to the beach.

Rene.
-

From: WU Fengguang
Date: Monday, August 13, 2007 - 12:38 am

Sorry, I overlooked this chunk in int insert_vm_struct(mm, vma), hehe.

-

From: Alan Cox
Date: Monday, August 13, 2007 - 6:01 am

On Mon, 13 Aug 2007 15:38:53 +0800

Ok please apply the patch then Andrew

---

The new exec code inserts an accounted vma into an mm struct which is not
current->mm. The existing memory check code has a hard coded assumption
that this does not happen as does the security code.

As the correct mm is known we pass the mm to the security method and the
helper function. A new security test is added for the case where we need
to pass the mm and the existing one is modified to pass current->mm to
avoid the need to change large amounts of code.

Signed-off-by: Alan Cox <alan@redhat.com>


diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc1-mm1/include/linux/mm.h linux-2.6.23rc1-mm1/include/linux/mm.h
--- linux.vanilla-2.6.23rc1-mm1/include/linux/mm.h	2007-07-26 15:02:58.000000000 +0100
+++ linux-2.6.23rc1-mm1/include/linux/mm.h	2007-08-12 13:54:24.614647536 +0100
@@ -1079,7 +1079,7 @@
 }
 
 /* mmap.c */
-extern int __vm_enough_memory(long pages, int cap_sys_admin);
+extern int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin);
 extern void vma_adjust(struct vm_area_struct *vma, unsigned long start,
 	unsigned long end, pgoff_t pgoff, struct vm_area_struct *insert);
 extern struct vm_area_struct *vma_merge(struct mm_struct *,
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc1-mm1/include/linux/security.h linux-2.6.23rc1-mm1/include/linux/security.h
--- linux.vanilla-2.6.23rc1-mm1/include/linux/security.h	2007-07-26 15:02:58.000000000 +0100
+++ linux-2.6.23rc1-mm1/include/linux/security.h	2007-08-12 14:13:10.383504656 +0100
@@ -58,7 +58,7 @@
 extern int cap_task_setioprio (struct task_struct *p, int ioprio);
 extern int cap_task_setnice (struct task_struct *p, int nice);
 extern int cap_syslog (int type);
-extern int cap_vm_enough_memory (long pages);
+extern int cap_vm_enough_memory (struct mm_struct *mm, long pages);
 
 struct msghdr;
 struct sk_buff;
@@ -1129,6 +1129,7 @@
  ...
From: Andrew Morton
Date: Monday, August 13, 2007 - 10:01 pm

erp, we have major version skew between 2.6.23-rc1-mm1 and 2.6.23-rc3 here, sorry.

Could I ask for a redone patch against mainline?

Thanks.
-

From: Tobias Diedrich
Date: Tuesday, August 14, 2007 - 10:50 am

Since I'm seeing this problem on 2.6.23-rc3, I tried adapting it.
I'm running it now and the NULL pointer messages are gone.

Index: linux-2.6.23-rc3/include/linux/mm.h
===================================================================
--- linux-2.6.23-rc3.orig/include/linux/mm.h	2007-08-14 18:33:09.000000000 +0200
+++ linux-2.6.23-rc3/include/linux/mm.h	2007-08-14 19:31:59.000000000 +0200
@@ -1042,7 +1042,7 @@
 }
 
 /* mmap.c */
-extern int __vm_enough_memory(long pages, int cap_sys_admin);
+extern int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin);
 extern void vma_adjust(struct vm_area_struct *vma, unsigned long start,
 	unsigned long end, pgoff_t pgoff, struct vm_area_struct *insert);
 extern struct vm_area_struct *vma_merge(struct mm_struct *,
Index: linux-2.6.23-rc3/include/linux/security.h
===================================================================
--- linux-2.6.23-rc3.orig/include/linux/security.h	2007-08-14 18:33:10.000000000 +0200
+++ linux-2.6.23-rc3/include/linux/security.h	2007-08-14 19:38:24.000000000 +0200
@@ -54,7 +54,7 @@
 extern int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid, int flags);
 extern void cap_task_reparent_to_init (struct task_struct *p);
 extern int cap_syslog (int type);
-extern int cap_vm_enough_memory (long pages);
+extern int cap_vm_enough_memory (struct mm_struct *mm, long pages);
 
 struct msghdr;
 struct sk_buff;
@@ -1125,6 +1125,7 @@
  *	Return 0 if permission is granted.
  * @vm_enough_memory:
  *	Check permissions for allocating a new virtual mapping.
+ *	@mm contains the mm struct it is being added to.
  *      @pages contains the number of pages.
  *	Return 0 if permission is granted.
  *
@@ -1169,7 +1170,7 @@
 	int (*quota_on) (struct dentry * dentry);
 	int (*syslog) (int type);
 	int (*settime) (struct timespec *ts, struct timezone *tz);
-	int (*vm_enough_memory) (long pages);
+	int (*vm_enough_memory) (struct mm_struct *mm, long pages);
 
 	int ...
Previous thread: Re: [PATCH] pata_artop: fix UDMA5 for AEC6280[R] and UDMA6 for AEC6880[R] by Mikael Pettersson on Sunday, August 12, 2007 - 4:53 am. (2 messages)

Next thread: Improving read/write/close system call reliability when used with pthreads by Fredrik Noring on Sunday, August 12, 2007 - 6:03 am. (9 messages)