kern/54418: Bug in VM page protection handling.

[Pawel Jakub Dawidek]

>Number:         54418
>Category:       kern
>Synopsis:       Bug in VM page protection handling.
>Confidential:   no
>Severity:       non-critical
>Priority:       high
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Jul 12 13:10:13 PDT 2003
>Closed-Date:
>Last-Modified:
>Originator:     Pawel Jakub Dawidek
>Release:        FreeBSD 4.x, FreeBSD 5.x.
>Organization:
Pawel Jakub Dawidek
>Environment:
System: FreeBSD milla.ask33.net 4.8-RELEASE FreeBSD 4.8-RELEASE #1: Mon Apr 7 09:37:03 CEST 2003 root at milla.ask33.net:/usr/obj/usr/src/sys/MILLA i386
All versions of FreeBSD 4.x and FreeBSD 5.x.

	
>Description:
	There is a problem in setting page protection in function
	vm_map_protect().
	When we set for example max_protection to VM_PROT_READ and
	after that we will try do change max_protection to VM_PROT_ALL
	there is no chance to do that, because of bogus check. This 'if'
	doesn't check if we set max_protection or just protection and
	denieds all increasing max_protection tries.
	Problem doesn't affect FreeBSD directly, because such situation
	never occurs, but for 3rd-party kernel modules this could be importent.
	For example, for my module - cerb - where I need to do operations
	on VM pages, it is very important and this bug provoke 'Bus error's
	in some situations.
>How-To-Repeat:
	This sample kernel module shows the problem. It is for FreeBSD 4.x,
	but simlar could be prepared for 5.x.

#include <sys/param.h>

#include <sys/proc.h>
#include <sys/kernel.h>
#include <sys/module.h>
#include <sys/systm.h>

#include <sys/lock.h>
#include <vm/vm.h>
#include <vm/pmap.h>
#include <vm/vm_map.h>

static int
mod(struct module *module, int cmd, void *arg)
{
	vm_map_t map = &curproc->p_vmspace->vm_map;
	vm_offset_t start, end;
	int error = 0;

	switch (cmd) {
	case MOD_LOAD:
		end = start = (vm_offset_t)curproc->p_vmspace->vm_daddr +
		    ctob(curproc->p_vmspace->vm_dsize);
		start--;
		error = vm_map_protect(map, start, end, VM_PROT_READ, TRUE);
		printf("ERROR1: %d\n", error);
		error = vm_map_protect(map, start, end, VM_PROT_ALL, TRUE);
		/* Here should be 2 which means KERN_PROTECTION_FAILURE. */
		printf("ERROR2: %d\n", error);
		error = 0;
		printf("testmod loaded.\n");
		break;
	case MOD_UNLOAD:
		printf("testmod unloaded.\n");
		break;
	default:
		error = EINVAL;
		break;
	}

	return (error);
}

static moduledata_t testmod_mod =
{
	"testmod",
	mod,
	NULL
};

DECLARE_MODULE(testmod, testmod_mod, SI_SUB_DRIVERS, SI_ORDER_MIDDLE);

>Fix:
	This patch fix this.

diff -upr /usr/src/sys/vm/vm_map.c src/sys/vm/vm_map.c
--- /usr/src/sys/vm/vm_map.c	Fri Jul  4 00:38:04 2003
+++ src/sys/vm/vm_map.c	Sat Jul 12 21:15:25 2003
@@ -1393,7 +1393,8 @@ vm_map_protect(vm_map_t map, vm_offset_t
 			vm_map_unlock(map);
 			return (KERN_INVALID_ARGUMENT);
 		}
-		if ((new_prot & current->max_protection) != new_prot) {
+		if (!set_max &&
+		    (new_prot & current->max_protection) != new_prot) {
 			vm_map_unlock(map);
 			return (KERN_PROTECTION_FAILURE);
 		}
>Release-Note:
>Audit-Trail:
>Unformatted: