Re: [ddraw] Fix bug 3487 take 2

[Peter Berg Larsen]

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