[ddraw] Fix bug 3487 take 2

Lionel Ulmer lionel.ulmer at free.fr
Mon Oct 10 14:15:28 CDT 2005


On Mon, Oct 10, 2005 at 01:43:04AM +0200, Peter Berg Larsen wrote:
> > Could you try it and tell if it fixes the problem ?
> 
> It, the patch on the bug page, does (e.i. without the locking). Could I 
> suggest a comment in the code.

Well, you can suggest a comment or even send a patch adding comments if you
wish :-)

And as my patch is in CVS now, you can even do a regression test with the
'full blown' patch.

> Nor did I say that; just that it had nothing to with bug 3487 as the
> subject said it had.

Yeah, got confused too (it really took me a while to understand that we went
out of the Lock function before crashing).

Now back to your patch. The code looks like this:

/* __tosize can be set too large by some programs */
#define DD_STRUCT_COPY_BYSIZE(to,from)
do {
    DWORD __tosize = (to)->dwSize;
    DWORD __fromsize = (from)->dwSize;
    if ((to) == (from))
        break;
    if (__tosize > sizeof(*(to)))
        ERR("To struct's size too large");
    if (__fromsize > sizeof(*(from)))
        ERR("From struct's size too large");
    if (__fromsize > __tosize)
        ERR("Copying too large struct");
    memcpy(to,from,__fromsize);
    memset(to+__fromsize,0,sizeof(*(to))-__fromsize);    
    (to)->dwSize = __tosize;/*restore size*/
} while (0)

I would first merge both our patches replacing the following lines with an
'assert(to != from)':

    if ((to) == (from))
        break;

This macro is only used to copy application data to Wine data or the
reverse. So if both pointers are the same, it is a Wine bug as we should
NEVER send any pointer to our private data to the application (they are not
const pointers => the application can modify them => all hell break loose).

After you also give a lot of warnings but you do not do as the previous
macro. Ie if 'fromsize' is bigger than 'tosize', you just print an error,
you do not 'correct' the size which would lead to a memory overflow.

Finally, 'to+__fromsize' is wrong, it should be '((char *) to)+__fromsize'
and why use 'sizeof(*(to))' as the basis for the 'memset' size and not
'tosize' ?

         Lionel

-- 
		 Lionel Ulmer - http://www.bbrox.org/



More information about the wine-devel mailing list