Regression: Winrar fails to start

[Dustin Navea]

Can I get someone to take a look at 
http://bugs.winehq.org/show_bug.cgi?id=3573 ?  This seems to affect only 
  users of Xorg, and it seems to affect all of them, in multiple 
different programs..

Dustin

[Christian Lachner]

Also got that Problem with Audiograbber... Other programs are surely also
affected by this problem

On 11/9/05, Dustin Navea  wrote:
> Can I get someone to take a look at
> http://bugs.winehq.org/show_bug.cgi?id=3573 ? This seems to affect only
> users of Xorg, and it seems to affect all of them, in multiple
> different programs..
>
> Dustin

[Marcus Meissner]

On Wed, Nov 09, 2005 at 04:24:43PM +0100, Christian Lachner wrote:
> Also got that Problem with Audiograbber... Other programs are surely also
> affected by this problem
>
> On 11/9/05, Dustin Navea  wrote:
> > Can I get someone to take a look at
> > http://bugs.winehq.org/show_bug.cgi?id=3573 ? This seems to affect only
> > users of Xorg, and it seems to affect all of them, in multiple
> > different programs..

How is Audiograbber affected?

The problem is a X11 security fix, which disallows
width or height of Pixmaps to be > 32768.

The goal of the original security fix is to avoid integer overflows
in multiplication of width and height.

(The fix is in the lines of:
    if ((width > 32768) || (height > 32768=))
    	Error();
)

I already mail [e-mail] with this problem ...

Should I put effort in to get it fixed better?

Ciao, Marcus

[Christian Lachner]

Hmm, i thought you were talking about the BadAlloc-Problem and i noticed
that audiograbber does not start because of that. Did i misunderstand
something? I'm not a dev so i don't understand your fix...

ciao chris

On 11/9/05, Marcus Meissner  wrote:
> On Wed, Nov 09, 2005 at 04:24:43PM +0100, Christian Lachner wrote:
> > Also got that Problem with Audiograbber... Other programs are surely
>
> also
> > affected by this problem
> >
> > On 11/9/05, Dustin Navea  wrote:
> > > Can I get someone to take a look at
> > > http://bugs.winehq.org/show_bug.cgi?id=3573 ? This seems to affect
>
> only
> > > users of Xorg, and it seems to affect all of them, in multiple
> > > different programs..
>
> How is Audiograbber affected?
>
> The problem is a X11 security fix, which disallows
> width or height of Pixmaps to be > 32768.
>
> The goal of the original security fix is to avoid integer overflows
> in multiplication of width and height.
>
> (The fix is in the lines of:
> if ((width > 32768) || (height > 32768=))
> Error();
> )
>
> I already mail [e-mail] with this problem ...
>
> Should I put effort in to get it fixed better?
>
> Ciao, Marcus

[Dustin Navea]

Yes, we are talking about the same thing, it is due to a bug in X that 
was fixed.  The X fix broke wine, causing that error to come up, so we 
are now trying to figure out how to get around it or get X fixed without 
breaking wine.

Christian Lachner wrote:
> Hmm, i thought you were talking about the BadAlloc-Problem and i noticed 
> that audiograbber does not start because of that. Did i misunderstand 
> something? I'm not a dev so i don't understand your fix...
>
> ciao chris
>
> On 11/9/05, *Marcus Meissner* <[e-mail] 
> > wrote:
> On Wed, Nov 09, 2005 at 04:24:43PM +0100, Christian Lachner wrote:
>  > Also got that Problem with Audiograbber... Other programs are
> surely also
>  > affected by this problem
>  >
>  > On 11/9/05, Dustin Navea < [e-mail]
> <mailto:[e-mail]>> wrote:
>  > >
>  > > Can I get someone to take a look at
>  > > http://bugs.winehq.org/show_bug.cgi?id=3573
> <http://bugs.winehq.org/show_bug.cgi?id=3573> ? This seems to affect
> only
>  > > users of Xorg, and it seems to affect all of them, in multiple
>  > > different programs..
>
> How is Audiograbber affected?
>
> The problem is a X11 security fix, which disallows
> width or height of Pixmaps to be > 32768.
>
> The goal of the original security fix is to avoid integer overflows
> in multiplication of width and height.
>
> (The fix is in the lines of:
>         if ((width > 32768) || (height > 32768=))
>                 Error();
> )
>
> I already mail [e-mail] <mailto:[e-mail]> with
> this problem ...
>
> Should I put effort in to get it fixed better?
>
> Ciao, Marcus

[Dustin Navea]

Well, I was thinking that since we really dont want to mess with Xorg 
security fixes, maybe there is some sort of a workaround that could be 
written into wine?  I dunno, perhaps a temporary dirty hack to fix it 
until Xorg fixes X again?

Unless there isnt any way to work around it of course, and then we just 
have to wait on them to see what they say.

Dustin

Marcus Meissner wrote:
> On Wed, Nov 09, 2005 at 04:24:43PM +0100, Christian Lachner wrote:
> > Also got that Problem with Audiograbber... Other programs are surely also
> > affected by this problem
> >
> > On 11/9/05, Dustin Navea  wrote:
> > > Can I get someone to take a look at
> > > http://bugs.winehq.org/show_bug.cgi?id=3573 ? This seems to affect only
> > > users of Xorg, and it seems to affect all of them, in multiple
> > > different programs..
>
> How is Audiograbber affected?
>
> The problem is a X11 security fix, which disallows
> width or height of Pixmaps to be > 32768.
>
> The goal of the original security fix is to avoid integer overflows
> in multiplication of width and height.
>
> (The fix is in the lines of:
>    if ((width > 32768) || (height > 32768=))
>    	Error();
> )
>
> I already mail [e-mail] with this problem ...
>
> Should I put effort in to get it fixed better?
>
> Ciao, Marcus

On Thu, 10 Nov 2005 06:02:57 +0100, Dustin Navea   
wrote:
> Well, I was thinking that since we really dont want to mess with Xorg  
> security fixes, maybe there is some sort of a workaround that could be  
> written into wine?  I dunno, perhaps a temporary dirty hack to fix it  
> until Xorg fixes X again?
>
> Unless there isnt any way to work around it of course, and then we just  
> have to wait on them to see what they say.
> > The problem is a X11 security fix, which disallows
> > width or height of Pixmaps to be > 32768.
> >  The goal of the original security fix is to avoid integer overflows
> > in multiplication of width and height.
> >  (The fix is in the lines of:
> >    if ((width > 32768) || (height > 32768=))
> >    	Error();
> > )
> >  I already mail [e-mail] with this problem ...  Should I put  
> > effort in to get it fixed better?
> >  Ciao, Marcus

It seems that that "security fix" in xorg is very poorly thought out. I  
also wonder how much of a risk that really is. Do you have bug ref for it?

What ever is going on it must be sorted out in xorg not in wine so if you  
want to get involved try proposing a better xorg patch than that clumbsy  
hack. ;)

Wine has enuf to do without chasing that kind of silliness elsewhere.

regards.

On Thu, 10 Nov 2005 06:02:57 +0100, Dustin Navea   
wrote:
> x.org

Seems there are multiple issues here. First the x.org bug was big and ugly  
so the rather simplistic patch was put through until what looks like some  
sloppy coding in the rest of X gets cleaned up.

http://www.x.org/pub/X11R6.8.2/patches/xorg-CAN-2005-2495.patch -

+    if (stuff->width > 32767 || stuff->height > 32767)
+    {
+/* It is allowed to try and allocate a pixmap which is larger than
+ * 32767 in either dimension. However, all of the framebuffer code
+ * is buggy and does not reliably draw to such big pixmaps, basically
+ * because the Region data structure operates with signed shorts
+ * for the rectangles in it.
+ *
+ * Furthermore, several places in the X server computes the
+ * size in bytes of the pixmap and tries to store it in an
+ * integer. This integer can overflow and cause the allocated size
+ * to be much smaller.
+ *
+ * So, such big pixmaps are rejected here with a BadAlloc
+ */
+return BadAlloc;

Frankly I'm shocked.
And appologies to the creator of that patch for my having made comment on  
the basis of what was posted here.

from
http://bugs.winehq.org/show_bug.cgi?id=3573

It seems to be that WinRAR crashes after calling:
ImageList_Create(16,16,ILC_COLOR32|ILC_MASK,256,2048)
to create the list of icons for ListView control. Wine translates
this ImageList_Create to CreateBitmapIndirect 36864x16 and then
cannot handle 36864 parameter. If it is true, I think, it should be
fixed in Wine. I cannot find any errors in WinRAR ImageList_Create call.

Well if that's the case there is certainly something that can be done in  
Wine.

It seems very odd that a simple thing like Winrar should need a 36k wide  
bitmap!

If this is coming up on imageList_Create there are sure to be other  
victims.

regards.

[Robert Shearman]

[e-mail] wrote:
> It seems to be that WinRAR crashes after calling:
> ImageList_Create(16,16,ILC_COLOR32|ILC_MASK,256,2048)
> to create the list of icons for ListView control. Wine translates
> this ImageList_Create to CreateBitmapIndirect 36864x16 and then
> cannot handle 36864 parameter. If it is true, I think, it should be
> fixed in Wine. I cannot find any errors in WinRAR ImageList_Create call.

Wine isn't really doing anything wrong, so there is nothing to be fixed. 
However, it seems that image lists have a feature of somwhat-lazy 
initialisation - they can an initial image count and a max image count. 
It seems we could use this feature to only allocate the initial image 
count on creation and allocate up to the maximum if needed when more 
than the initial images are added.

[Peter Beutner]

Robert Shearman schrieb:
> [e-mail] wrote:
> > It seems to be that WinRAR crashes after calling:
> > ImageList_Create(16,16,ILC_COLOR32|ILC_MASK,256,2048)
> > to create the list of icons for ListView control. Wine translates
> > this ImageList_Create to CreateBitmapIndirect 36864x16 and then
> > cannot handle 36864 parameter. If it is true, I think, it should be
> > fixed in Wine. I cannot find any errors in WinRAR ImageList_Create call.
>
> Wine isn't really doing anything wrong, so there is nothing to be fixed.

difficult to say who is doing wrong here ;)
you could also say wine wrongly uses the X11/Xlib api :p
> However, it seems that image lists have a feature of somwhat-lazy
> initialisation - they can an initial image count and a max image count.
> It seems we could use this feature to only allocate the initial image
> count on creation and allocate up to the maximum if needed when more
> than the initial images are added.

That doesn't really fix the problem. It probably just crashes later when the ImageList has
grown enough. Neither it fixes the case when an app requests an ImageList with an initial
image count high enough to trigger the BadAlloc failure.

The only solution I can think of is to implement that ImageList stuff using some kind of
linked list of pixmaps.(Though I don't know that API at all and don't know if it is
required that all images should be packed together in memory ??)

[Vitaliy Margolen]

Thursday, November 10, 2005, 2:59:02 PM, Robert Shearman wrote:
> [e-mail] wrote:
> > It seems to be that WinRAR crashes after calling:
> > ImageList_Create(16,16,ILC_COLOR32|ILC_MASK,256,2048)
> > to create the list of icons for ListView control. Wine translates
> > this ImageList_Create to CreateBitmapIndirect 36864x16 and then
> > cannot handle 36864 parameter. If it is true, I think, it should be
> > fixed in Wine. I cannot find any errors in WinRAR ImageList_Create call.
>
> Wine isn't really doing anything wrong, so there is nothing to be fixed.
> However, it seems that image lists have a feature of somwhat-lazy 
> initialisation - they can an initial image count and a max image count. 
> It seems we could use this feature to only allocate the initial image 
> count on creation and allocate up to the maximum if needed when more 
> than the initial images are added.

The real fix would be is to store images in the image list the same way native
does it (4 images in a row, growing down). Wine stores them flat (n x 1).

Vitaliy

[Dmitry Timoshkov]

 wrote:
> http://www.x.org/pub/X11R6.8.2/patches/xorg-CAN-2005-2495.patch -
> - if (stuff->width > 32767 || stuff->height > 32767)
> - {
>
> +/* It is allowed to try and allocate a pixmap which is larger than
> -
> - 32767 in either dimension. However, all of the framebuffer code
> -
> - is buggy and does not reliably draw to such big pixmaps, basically
> -
> - because the Region data structure operates with signed shorts
> -
> - for the rectangles in it.
> -
> -
> -
> - Furthermore, several places in the X server computes the
> -
> - size in bytes of the pixmap and tries to store it in an
> -
> - integer. This integer can overflow and cause the allocated size
> -
> - to be much smaller.
> -
> -
> -
> - So, such big pixmaps are rejected here with a BadAlloc
> - */
>
> +return BadAlloc;

This clearly indicates the places X server sources which need to be fixed.
Carefully reread very first sentence if still in doubt.
> Frankly I'm shocked.
> And appologies to the creator of that patch for my having made comment on  
> the basis of what was posted here.
>
> from
> http://bugs.winehq.org/show_bug.cgi?id=3573
>
> It seems to be that WinRAR crashes after calling:
> ImageList_Create(16,16,ILC_COLOR32|ILC_MASK,256,2048)
> to create the list of icons for ListView control. Wine translates
> this ImageList_Create to CreateBitmapIndirect 36864x16 and then
> cannot handle 36864 parameter. If it is true, I think, it should be
> fixed in Wine. I cannot find any errors in WinRAR ImageList_Create call.

Wine ought to be able to handle images larger than 32767x32767, that's not
a Wine requirement, that's what applications written for win32 API expect
to see working. There is no way to make it work without DIB engine if X11
doesn't support it due to ineternal bugs.

[Alexandre Julliard]

"Dmitry Timoshkov"  writes:
> Wine ought to be able to handle images larger than 32767x32767, that's not
> a Wine requirement, that's what applications written for win32 API expect
> to see working. There is no way to make it work without DIB engine if X11
> doesn't support it due to ineternal bugs.

In general yes, but in this case it's Wine that is creating the huge
bitmap, and that could easily be avoided.