window-maker / wmaker

Mirror of the official repository repo.or.cz/wmaker-crm.git. Do not send pull requests here, send your patches to wmaker-dev@googlegroups.com instead
https://repo.or.cz/wmaker-crm.git
GNU General Public License v2.0
136 stars 18 forks source link

crash due to negative height. #7

Closed jnse closed 3 years ago

jnse commented 3 years ago

I can reliably reproduce this when i start tint2 in windowmaker, and then launch a few additional windows, when using an asymmetric 4-monitor layout.

What happens is that, in placement.c:176, inside the PlaceIcon function, somehow sh ends up with a negative number.

This number is then passed into wmalloc with: map = wmalloc((sw + 2) * (sh + 2));

Ouch. So you end up multiplying with a negative number, and when the signed int turns unsigned, you get a crazy high number, the malloc fails consistently, and subsequently wmaker crashes.

I managed to hack around this in my local copy by adding a pair of conditionals above the malloc line:

    if (sh < 0) sh = -1; 
    if (sw < 0) sw = -1; 

(-1 since 2 is added to it in the wmalloc call, so you end up with a value of 1 in the worst case)

tl;dr there should be sanity checks before malloc'ing.

gryf commented 3 years ago

Thanks for the report! can you provide your 4-monitor layout (dimensions) and a scenario, when you are bumping on this?

jnse commented 3 years ago

Sure, here's an arandr screenshot, and the xrandr output below:

arandr

Screen 0: minimum 8 x 8, current 2944 x 1980, maximum 32767 x 32767
DP-0 disconnected primary (normal left inverted right x axis y axis)
DP-1 connected 1600x900+320+0 (normal left inverted right x axis y axis) 443mm x 249mm
   1600x900      59.95*+  60.00    59.95    59.82  
   2048x1152     59.91  
   1920x1200     59.95  
   1920x1080     59.93  
   1680x1050     59.95    59.88  
   1400x1050     59.98  
   1400x900      59.96    59.88  
   1368x768      59.88    59.85  
   1280x1024     75.02    60.02  
   1280x960      60.00  
   1280x800      59.91    59.81  
   1280x720      60.00    59.86    59.74  
   1152x864      75.00  
   1024x768      75.03    70.07    60.00  
   1024x576      59.90    59.82    59.96    59.95  
   960x600       60.00    59.93  
   960x540       59.82    59.63    59.99    59.96  
   864x486       59.92    59.57  
   840x525       60.01    59.88  
   832x624       74.55  
   800x600       75.00    72.19    60.32    56.25    60.00  
   800x450       59.95    59.82  
   700x525       59.98  
   700x450       59.96    59.88  
   684x384       59.88    59.85  
   640x512       75.02    60.02  
   640x480       75.00    72.81    72.81    59.94    60.00  
   640x400       59.98    59.88  
   640x360       59.86    59.83  
   576x432       75.00  
   512x384       75.03    70.07    60.00  
   512x288       60.00    59.92  
   480x270       59.82    59.63  
   432x243       59.92    59.57  
   416x312       74.66  
   400x300       75.12    72.19    60.32    56.34  
   320x240       75.00    72.81    60.05  
HDMI-0 connected 1920x1080+0+900 (normal left inverted right x axis y axis) 1152mm x 648mm
   1920x1080     60.00*+  59.94    59.93    50.00    29.97    25.00    23.98  
   1680x1050     59.95    59.88  
   1600x900      59.95    59.82  
   1400x1050     59.98  
   1400x900      59.96    59.88  
   1368x768      59.88    59.85  
   1360x768      59.80  
   1280x1024     60.02  
   1280x960      60.00  
   1280x800      59.91    59.81  
   1280x720      60.00    60.00    59.94    59.86    59.74  
   1152x864      75.00  
   1024x768      75.03    70.07    60.00  
   1024x576      59.90    59.82  
   960x540       59.82    59.63    59.99    59.96  
   864x486       59.92    59.57  
   840x525       60.01    59.88  
   832x624       74.55  
   800x600       75.00    72.19    60.32    56.25  
   800x450       59.95    59.82  
   720x576       50.00  
   720x480       59.94  
   720x405       59.51    58.99  
   700x525       59.98  
   700x450       59.96    59.88  
   684x384       59.88    59.85  
   640x512       60.02  
   640x480       75.00    72.81    59.93    59.94    60.00  
   640x400       59.98    59.88  
   640x360       59.84    59.32    59.86    59.83  
   576x432       75.00  
   512x384       75.03    70.07    60.00  
   512x288       60.00    59.92  
   480x270       59.82    59.63  
   432x243       59.92    59.57  
   416x312       74.66  
   400x300       75.12    72.19    60.32    56.34  
   360x202       59.51    59.13  
   320x240       75.00    72.81    60.05  
   320x180       59.84    59.32  
DP-2 connected 1024x768+1920+132 (normal left inverted right x axis y axis) 304mm x 228mm
   1024x768      60.00*+  75.03    70.07  
   1368x768      59.85  
   1280x800      59.91  
   1280x720      59.86    59.74  
   1024x576      59.90    59.82  
   960x540       59.82    59.63  
   864x486       59.92    59.57  
   832x624       74.55  
   800x600       75.00    72.19    60.32    56.25  
   800x450       59.95    59.82  
   700x450       59.96    59.88  
   684x384       59.88    59.85  
   640x480       75.00    72.81    72.81    59.94    60.00  
   640x400       59.98    59.88  
   640x360       59.86    59.83  
   512x384       75.03    70.07    60.00  
   512x288       60.00    59.92  
   480x270       59.82    59.63  
   432x243       59.92    59.57  
   416x312       74.66  
   400x300       75.12    72.19    60.32    56.34  
   320x240       75.00    72.81    60.05  
DP-3 disconnected (normal left inverted right x axis y axis)
DP-4 connected 1024x768+1920+900 (normal left inverted right x axis y axis) 1020mm x 570mm
   1024x768      60.00 +  85.00*   75.03    70.07    60.00    60.04  
   1920x1080     60.00    59.93  
   1680x1050     59.95    59.88  
   1600x900      60.00    59.95    59.82  
   1440x900      59.89  
   1400x1050     59.98    59.98  
   1400x900      59.96    59.88  
   1368x768      59.88    59.85  
   1366x768      59.79  
   1280x1024     75.02    60.02  
   1280x960      85.00    60.00  
   1280x800      59.91    59.81    59.81    59.97  
   1280x768      59.87  
   1280x720      60.00    59.86    59.74    59.99  
   1152x864      75.00    59.96  
   1024x576      59.90    59.82    59.96    59.95  
   960x720       75.00    60.00  
   960x600       60.00    59.93  
   960x540       59.82    59.63    59.99    59.96  
   928x696       75.00    60.05  
   896x672       75.05    60.01  
   864x486       59.92    59.57  
   840x525       60.01    59.88  
   832x624       74.55  
   800x600       85.14    75.00    72.19    60.32    56.25    85.00    75.00    70.00    65.00    60.00  
   800x450       59.95    59.82  
   720x400       85.04  
   700x525       74.76    59.98  
   700x450       59.96    59.88  
   684x384       59.88    59.85  
   640x512       85.02    75.02    60.02  
   640x480       85.01    75.00    72.81    72.81    59.94    85.09    60.00  
   640x400       85.08    59.98    59.88  
   640x360       59.86    59.83  
   640x350       85.08  
   576x432       75.00  
   512x384       85.00    75.03    70.07    60.00  
   512x288       60.00    59.92  
   480x270       59.82    59.63  
   432x243       59.92    59.57  
   416x312       74.66  
   400x300       85.27    75.12    72.19    60.32    56.34  
   360x200       85.04  
   320x240       85.18    75.00    72.81    60.05  
   320x200       85.27  
   320x175       85.27  
DP-5 disconnected (normal left inverted right x axis y axis)
USB-C-0 disconnected (normal left inverted right x axis y axis)

The different resolutions create quite a bit of deadspace, which I suspect is what might be related to the issue. I can reliably reproduce by launching tint2 and then subsequently creating a few terminal windows (after the 2nd or 3rd window, wmaker crashes because of the crazy malloc). I don't know what tint2 does that would cause negative numbers to go into the malloc, so that's a problem. But apart from that, there should be safeguards so that can never happen in the first place.

gryf commented 3 years ago

Yup, agreed. Thanks for the bug report. I'll propose the fix soon-ish :)

gryf commented 3 years ago

My investigation brings me to the conclusion, that tint2 set wrong dimensions (in my case it was height) and pass it as the information through to xorg, which in the end wmaker is reading. that's why you can observe negative height (and perhaps width) for certain layouts. For simple layouts and tint2 panel it works more or less as expected.

I've briefly checked same layout with openbox, and turns out, it doesn't care about panel at all - maximized windows are partially hidden behind the panel, which means, that openbox doesn't read those properties, or just ignores them.

jnse commented 3 years ago

Ah great; well, if that's the case, it sounds like the conditionals is all that's needed then to prevent the wmaker crash.

gryf commented 3 years ago

For sure we shouldn't malloc negative amount of memory, but perhaps it's worth to fill an issue to the tint2 project about wrong calculation. I'll give another look into this today, let see what I can find.

gryf commented 3 years ago

Ok, I finally figured it out. The culprit was in a way how we calculate the occupied area from _NET_WM_STRUT. For multihead setup, like you have, the left/right/top/bottom specified on this property has to be measured from the edge of the entire screen, not from the actual head. This is the reason, why we got the negative number for malloc, which comes from the coordinates of the usable area, here is the affected code: https://github.com/window-maker/wmaker/blob/224cb403a720574a8f824c6a658f326cb2c2f35f/src/wmspec.c#L848-L851 I'll propose the patch soon.

gryf commented 3 years ago

It has been fixed with bbf24d1 commit. @jnse could you test it on your environment?

jnse commented 3 years ago

Yes, no more crash now it seems.

crmafra commented 3 years ago

On Thu, 1 Apr 2021 at 18:10:50 -0700, John Sennesael wrote:

Yes, no more crash now it seems.

Great, thanks for reporting back. And thank you, Roman, for the fix!