Hi all,
Having lately some strange crashes when I start Wine with any --debugmsg command, I started to look at where the problem occured.
From my investigations, it came in the file misc/registry.c while loading
'system.1st' (yeah, I know, I should run in a no-Windows install, but well, my Wine installation is 6 years old and still working, so I won't change now :-) ).
Anyway, the faulty function is '_strdupnA'.
I added some traces and I have this :
0009:trace:reg:_strdupnA System\CurrentControlSet\Control\MediaProperties\PrivateProperties\Joystick (0x3c003f40) 79 0009:trace:reg:_strdupnA 0x3c003f90
This means that the string given as a parameter has the pointer '0x3c003f40', the length parameter is '79' and it returns '0x3c003f90'.
It crashes later on at :
0009:trace:reg:_strdupnA System\CurrentControlSet\Control\MediaProperties\PrivateProperties\Joystick\OEM (0x3c003f90) 114 0009:trace:reg:_strdupnA 0x3c003418
So we give the '0x3c003f90' pointer (which is of length 79 as seen in the previous debug output) but (if you look at the code), will read 114 bytes from it (as the function uses 'memcpy' and not 'strncpy').
The attached patch fixes all crashes on my box, but well, as I am not very familiar with this code, sent it to wine-devel and not wine-patches for review by Registry gurus :-)
Lionel
"Lionel Ulmer" lionel.ulmer@free.fr wrote:
The attached patch fixes all crashes on my box, but well, as I am not very familiar with this code, sent it to wine-devel and not wine-patches for review by Registry gurus :-)
I'm not a Registry guru by all means :-), but IMO the real bugs are at lines 701: new_key_name = _strdupnA(key_name,strlen(key_name)+dkh->keynamelen+1); and 1011: new_key_name = _strdupnA(key_name,strlen(key_name)+nk->name_len+1);
which incorrectly do request to copy too much bytes.
I'm not a Registry guru by all means :-), but IMO the real bugs are at lines 701: new_key_name = _strdupnA(key_name,strlen(key_name)+dkh->keynamelen+1); and 1011: new_key_name = _strdupnA(key_name,strlen(key_name)+nk->name_len+1);
Not really as the code does this :
/* create new subkey name */ new_key_name = _strdupnA(key_name,strlen(key_name)+dkh->keynamelen+1); if (strcmp(new_key_name,"") != 0) strcat(new_key_name,"\"); strncat(new_key_name,dkh->name,dkh->keynamelen); So basically it does 'duplicate my string but add XXX bytes to it as I want to strcat to it a new string of len XXX'.
Lionel
"Lionel Ulmer" lionel.ulmer@free.fr wrote:
Not really as the code does this :
/* create new subkey name */ new_key_name = _strdupnA(key_name,strlen(key_name)+dkh->keynamelen+1); if (strcmp(new_key_name,"") != 0) strcat(new_key_name,"\\"); strncat(new_key_name,dkh->name,dkh->keynamelen);
So basically it does 'duplicate my string but add XXX bytes to it as I want to strcat to it a new string of len XXX'.
It's clearly a bug, since the code asks for trouble by requesting to read more data than it actually should. In that case the code has to do:
new_key_name = malloc(strlen(key_name)+dkh->keynamelen+1); strcpy(new_key_name, key_name); if (strcmp(new_key_name,"") != 0) strcat(new_key_name,"\"); strncat(new_key_name,dkh->name,dkh->keynamelen);
It's clearly a bug, since the code asks for trouble by requesting to read more data than it actually should. In that case the code has to do:
new_key_name = malloc(strlen(key_name)+dkh->keynamelen+1); strcpy(new_key_name, key_name);
Well, these two lines are EXACTLY what the strndup code does :-)
Lionel
"Lionel Ulmer" lionel.ulmer@free.fr wrote:
It's clearly a bug, since the code asks for trouble by requesting to read more data than it actually should. In that case the code has to do:
new_key_name = malloc(strlen(key_name)+dkh->keynamelen+1); strcpy(new_key_name, key_name);
Well, these two lines are EXACTLY what the strndup code does :-)
I don't think so. memcpy != strcpy.
new_key_name = malloc(strlen(key_name)+dkh->keynamelen+1); strcpy(new_key_name, key_name);
Well, these two lines are EXACTLY what the strndup code does :-)
I don't think so. memcpy != strcpy.
I meant to add 'with my fix attached to my first mail'.
But well, will let the masters fix it (I am currently happpy with the fix I have in my tree).
Lionel