[MPlayer-dev-eng] [PATCH] (loader/ext.c) VirtualAlloc() SIGSEGV on Linux + other fixes

A. Guru a.guru at sympatico.ca
Mon Aug 16 11:22:54 CEST 2004


Changes:

    * mplayer received a SIGSEGV under Linux after

	Opening video decoder: [dmo] DMO video codecs
	VirtualAlloc(0x00400000, 859648, 0x00003000, 0x00000040)

      with

	-rw-r--r--  1 root root 807032 Nov 20  2002 /usr/local/lib/codecs/wmv9dmod.dll

      because that region was already under use and mmap() with MAP_FIXED has
      problems under Linux (see code).  VirtualAlloc() fixed in "loader/ext.c".

    * VirtualAlloc() made to conform with win32 documented behavior regarding
      the alignment of the address and size arguments.

    * VirtualAlloc() detection of overlap with previous regions fixed.



Feel free to comment printf() statements after proper behavior is verified.
Also feel free to ajust indentation style to your liking.
-------------- next part --------------
Changes:

    * mplayer received a SIGSEGV under Linux after

	Opening video decoder: [dmo] DMO video codecs
	VirtualAlloc(0x00400000, 859648, 0x00003000, 0x00000040)

      with

	-rw-r--r--  1 root root 807032 Nov 20  2002 /usr/local/lib/codecs/wmv9dmod.dll

      because that region was already under use and mmap() with MAP_FIXED has
      problems under Linux (see code).  VirtualAlloc() fixed in "loader/ext.c".

    * VirtualAlloc() made to conform with win32 documented behavior regarding
      the alignment of the address and size arguments.

    * VirtualAlloc() detection of overlap with previous regions fixed.



Feel free to comment printf() statements after proper behavior is verified.
Also feel free to ajust indentation style to your liking.



--- loader/ext.c.orig-1.0pre5	2002-09-13 15:43:13 -0400
+++ loader/ext.c	2004-08-16 04:29:50 -0400
@@ -457,13 +457,32 @@ static virt_alloc* vm=0;
 LPVOID WINAPI VirtualAlloc(LPVOID address, DWORD size, DWORD type,  DWORD protection)
 {
     void* answer;
-    int fd=open("/dev/zero", O_RDWR);
+    int fd;
+    long pgsz;
+
+    printf("VirtualAlloc(0x%08X, %u, 0x%08X, 0x%08X)\n", (unsigned)address, size, type, protection);
+
+    if ((type&(MEM_RESERVE|MEM_COMMIT)) == 0) return NULL;
+
+    fd=open("/dev/zero", O_RDWR);
     if(fd<0){
         perror( "Cannot open /dev/zero for READ+WRITE. Check permissions! error: " );
 	return NULL;
     }
-    size=(size+0xffff)&(~0xffff);
-    //printf("VirtualAlloc(0x%08X, %d)\n", address, size);
+
+    if (type&MEM_RESERVE && (unsigned)address&0xffff) {
+	size += (unsigned)address&0xffff;
+	(unsigned)address &= ~0xffff;
+    }
+    pgsz = sysconf(_SC_PAGESIZE);
+    if (type&MEM_COMMIT && (unsigned)address%pgsz) {
+	size += (unsigned)address%pgsz;
+	address -= (unsigned)address%pgsz;
+    }
+
+    if (type&MEM_RESERVE && size<0x10000) size = 0x10000;
+    if (size%pgsz) size += pgsz - size%pgsz;
+
     if(address!=0)
     {
     //check whether we can allow to allocate this
@@ -475,7 +494,7 @@ LPVOID WINAPI VirtualAlloc(LPVOID addres
 		str=str->prev;
 		continue;
 	    }
-	    if((unsigned)address+size<(unsigned)str->address)
+	    if((unsigned)address+size<=(unsigned)str->address)
 	    {
 		str=str->prev;
 		continue;
@@ -483,29 +502,40 @@ LPVOID WINAPI VirtualAlloc(LPVOID addres
 	    if(str->state==0)
 	    {
 #warning FIXME
-		if(((unsigned)address+size<(unsigned)str->address+str->mapping_size) && (type & MEM_COMMIT))
+		if(   ((unsigned)address >= (unsigned)str->address)
+		   && ((unsigned)address+size<=(unsigned)str->address+str->mapping_size)
+		   && (type & MEM_COMMIT))
 		{
 		    close(fd);
 		    return address; //returning previously reserved memory
 		}
-		return NULL;
+		printf(" VirtualAlloc(...) does not commit or not entirely within reserved, and\n");
 	    }
+	    printf(" VirtualAlloc(...) (0x%08X, %u) overlaps with (0x%08X, %u, state=%d)\n",
+	           (unsigned)address, size, (unsigned)str->address, str->mapping_size, str->state);
 	    close(fd);
 	    return NULL;
 	}
-	answer=mmap(address, size, PROT_READ | PROT_WRITE | PROT_EXEC,
-		    MAP_FIXED | MAP_PRIVATE, fd, 0);
     }
-    else
-	answer=mmap(address, size, PROT_READ | PROT_WRITE | PROT_EXEC,
-		    MAP_PRIVATE, fd, 0);
+
+    answer=mmap(address, size, PROT_READ | PROT_WRITE | PROT_EXEC,
+		MAP_PRIVATE, fd, 0);
 //    answer=FILE_dommap(-1, address, 0, size, 0, 0,
 //	PROT_READ | PROT_WRITE | PROT_EXEC, MAP_PRIVATE);
     close(fd);
+    if (answer != (void *)-1 && address && answer != address) {
+	/* It is dangerous to try mmap() with MAP_FIXED since it does not
+	   always detect conflicts or non-allocation and chaos ensues after
+	   a successful call but an overlapping or non-allocated region.  */
+	munmap(answer, size);
+	answer = (void *) -1;
+	errno = EINVAL;
+	printf(" VirtualAlloc(...) cannot satisfy requested address but address=NULL would work.\n");
+    }
     if(answer==(void*)-1)
     {
-	printf("Error no %d\n", errno);
-	printf("VirtualAlloc(0x%p, %ld) failed\n", address, size);
+	printf(" VirtualAlloc(...) mmap(0x%08X, %u, ...) failed with errno=%d (\"%s\")\n",
+	       (unsigned)address, size, errno, sys_errlist[errno]);
 	return NULL;
     }
     else
@@ -524,14 +554,17 @@ LPVOID WINAPI VirtualAlloc(LPVOID addres
 	vm->next=0;
 	//if(va_size!=0)
 	//    printf("Multiple VirtualAlloc!\n");
-	//printf("answer=0x%08x\n", answer);
+	printf(" VirtualAlloc(...) provides (0x%08X, %u)\n", (unsigned)answer, size);
         return answer;
     }
 }
+
 WIN_BOOL WINAPI VirtualFree(LPVOID  address, SIZE_T dwSize, DWORD dwFreeType)//not sure
 {
     virt_alloc* str=vm;
     int answer;
+
+    printf("VirtualFree(0x%08X, %d, 0x%08X)\n", (unsigned)address, dwSize, dwFreeType);
     while(str)
     {
 	if(address!=str->address)
@@ -539,7 +572,7 @@ WIN_BOOL WINAPI VirtualFree(LPVOID  addr
 	    str=str->prev;
 	    continue;
 	}
-	//printf("VirtualFree(0x%08X, %d - %d)\n", str->address, dwSize, str->mapping_size);
+	printf(" VirtualFree(...) munmap(0x%08X, %d)\n", (unsigned)str->address, str->mapping_size);
 	answer=munmap(str->address, str->mapping_size);
 	if(str->next)str->next->prev=str->prev;
 	if(str->prev)str->prev->next=str->next;


More information about the MPlayer-dev-eng mailing list