Re: patch for dlls/gdi32/dib.c: fixes crash

[Octavian Voicu]

On Wed, Jul 27, 2011 at 6:44 PM, Wolfgang Walter  wrote:
> - char src_bmibuf[FIELD_OFFSET( BITMAPINFO, bmiColors[256] )];
> - BITMAPINFO *src_info = (BITMAPINFO *)src_bmibuf;
> - char dst_bmibuf[FIELD_OFFSET( BITMAPINFO, bmiColors[256] )];
> - BITMAPINFO *dst_info = (BITMAPINFO *)dst_bmibuf;

There's another instance of that in GetDIBits. Allocating 2KB+ (2 x
1064 bytes) of data on the stack is not very reasonable I guess.

Octavian

patch for dlls/gdi32/dib.c: fixes crash

[Wolfgang Walter]

Am Mittwoch, 27. Juli 2011 schrieben Sie:
> On Wed, Jul 27, 2011 at 6:44 PM, Wolfgang Walter  wrote:
> > - char src_bmibuf[FIELD_OFFSET( BITMAPINFO, bmiColors[256] )];
> > - BITMAPINFO *src_info = (BITMAPINFO *)src_bmibuf;
> > - char dst_bmibuf[FIELD_OFFSET( BITMAPINFO, bmiColors[256] )];
> > - BITMAPINFO *dst_info = (BITMAPINFO *)dst_bmibuf;
>
> There's another instance of that in GetDIBits. Allocating 2KB+ (2 x
> 1064 bytes) of data on the stack is not very reasonable I guess.

Hmm, allocating the structure on the stack is not really the problem. The 
problem is that part if the stack gets overwritten.

Allocating the these structures on the heap hides the bug mostly.

I think that

bitmapinfo_from_user_bitmapinfo()

is the real culprit. I think colors gets to big (> 256) and therefore the size 
for the memcpy.

I just insert an test in bitmapinfo_from_user_bitmapinfo() if colors is larger 
then 256 and this is indeed the case.

Regards,

patch for dlls/gdi32/dib.c: fixes crash

[Huw Davies]

On Thu, Jul 28, 2011 at 04:14:56PM +0200, Wolfgang Walter wrote:
> I think that
>
> bitmapinfo_from_user_bitmapinfo()
>
> is the real culprit. I think colors gets to big (> 256) and therefore the size 
> for the memcpy.

Hopefully http://source.winehq.org/patches/data/77076 should fix this.

Huw.

patch for dlls/gdi32/dib.c: fixes crash

[Wolfgang Walter]

Am Donnerstag, 28. Juli 2011 schrieb Huw Davies:
> On Thu, Jul 28, 2011 at 04:14:56PM +0200, Wolfgang Walter wrote:
> > I think that
> >
> > bitmapinfo_from_user_bitmapinfo()
> >
> > is the real culprit. I think colors gets to big (> 256) and therefore the
> > size for the memcpy.
>
> Hopefully http://source.winehq.org/patches/data/77076 should fix this.

Yes, it does. Thanks.