[ddraw] Fix bug 3487 take 2

Peter Berg Larsen pebl at math.ku.dk
Mon Oct 10 15:18:21 CDT 2005



On Mon, 10 Oct 2005, Lionel Ulmer wrote:

>> 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).

Yep.


> /* __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;

I think thats execellent idea; I hadnt though of asserts, did not new 
wine used them..


> 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).

Hmm, ok. I read the comments bug 2070, as we only did the wine data to 
application data. Otherwise I think bug 2070 is missing handling too large 
from dwSize and could corrupt mem by memcpy.



> 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.

Yes. The latter 2, if ERR, should be asserts too. The first if ERR should 
be a WARN, as some applications breaks this assumption.


> Finally, 'to+__fromsize' is wrong, it should be '((char *) to)+__fromsize'

yes.


> and why use 'sizeof(*(to))' as the basis for the 'memset' size and not
> 'tosize' ?

Because tosize is not to be trusted by bug 2070, it could be to large. One
of invariants was that tosize >= sizeof(*to).

I still havent got the grasp of what dwSize is for if it does not reflect 
the size allocated?, nor the size of the struct current in the mem.


Peter



More information about the wine-devel mailing list