Complex dirty region causes overflow of xrdp comms buffer #318
Comments
|
@matt335672 @Nexarian @jsorg71 Can you reproduce this and have a look at this? |
|
I've just tried to reproduce and I've not managed it so far. I've got a breakpoint set on rdpClientCon.c:500 where the log message is generated. It's not firing. xrdp commit : d6fce2f17355ffb3c783ffb2a31c418b3e930b26 In startwm.sh I've just got I'm definitely negotiating GFX. Client is Windows 10. I'm probably missing something. Can you share a suitable |
|
An important point, "Use all monitors" and multimon is required. (I can reproduce with a single monitor though). This is likely to require at least a 4K screen size. diff --git a/sesman/sesman.ini.in b/sesman/sesman.ini.in
index 685af28b..7606f5d0 100644
--- a/sesman/sesman.ini.in
+++ b/sesman/sesman.ini.in
@@ -140,7 +140,7 @@ EnableSyslog=true
; CentOS 8 : param=/usr/libexec/Xorg
; FreeBSD (from 2022Q4) : param=/usr/local/libexec/Xorg
;
-param=Xorg
+param=/usr/libexec/Xorg
; Leave the rest parameters as-is unless you understand what will happen.
param=-config
param=xrdp/xorg.confHere is another Xorg log. |
|
Thanks. I'll have another go. I can probably set up a VM with two 4K monitors. |
|
I'm still not able to reproduce this with a Windows 10 client and two 4K monitors. In some ways I'm surprised this is happening, as with Could you post your I've tried with and without |
|
@matt335672 Ah, I made a serious typo. |
|
Thanks for the update! Following your new instructions I've reproduced this on a single 4K monitor. Stack trace is as follows:- I've worked out where the 'overrun' message is coming from. We call I've patched that as follows:- --- a/module/rdpClientCon.c
+++ b/module/rdpClientCon.c
@@ -1748,7 +1748,8 @@ rdpClientConPreCheck(rdpPtr dev, rdpClientCon *clientCon, int in_size)
rv = 1;
}
clientCon->count = 0;
- init_stream(clientCon->out_s, 0);
+ // Make sure stream is now big enough for the requested size
+ init_stream(clientCon->out_s, in_size + 20);
s_push_layer(clientCon->out_s, iso_hdr, 8);
}
@jsorg71 - could you check my logic? However, like you, I'm still seeing the X server hang up - I just don't get the message. So there seem to be two things going on here. I'll take another look Monday. |
|
rdpClientConPreCheck is to check if the new message can fit, if not, send what's there to make room. |
|
Understood - thanks. |
|
Here are some variables from a recent fail:-
There's a couple of observations here:-
|
|
I've made the following logging patch so I can see what the drects look like:- --- a/module/rdpClientCon.c
+++ b/module/rdpClientCon.c
@@ -2684,6 +2684,12 @@ rdpClientConSendPaintRectShmFd(rdpPtr dev, rdpClientCon *clientCon,
size += end_frame_bytes; /* end frame message */
size += 4; /* message 62 data_bytes */
+ if (size > (clientCon->out_s->size - 20))
+ {
+ // Way too big - pre-check will fail
+ LLOGLN(0, ("Size check prob: requested=%d max=%d", size, (clientCon->out_s->size - 20)));
+ }
+
rdpClientConPreCheck(dev, clientCon, size);
s = clientCon->out_s;
out_uint16_le(s, 62);
@@ -2840,6 +2846,16 @@ rdpCapRect(rdpClientCon *clientCon, BoxPtr cap_rect, struct image_data *id)
cap_dirty_save = rdpRegionCreate(NullBox, 0);
rdpRegionCopy(cap_dirty_save, cap_dirty);
num_rects = REGION_NUM_RECTS(cap_dirty);
+ if (num_rects > 2000)
+ {
+ BoxPtr rects_d = REGION_RECTS(cap_dirty);
+ int i;
+ LLOGLN(0, ("Dirty count = %d", num_rects));
+ for (i = 0; i < num_rects; ++i)
+ {
+ LLOGLN(0, ("dirty[%d] = (%d, %d, %d, %d)", i, rects_d[i].x1, rects_d[i].y1, rects_d[i].x2, rects_d[i].y2));
+ }
+ }
if (num_rects > 0)
{
rects = 0;Log file attached |
|
I can now see the problem, but I don't really understand the detail of this code. @jsorg71 - this really needs your expertise. When I can get --- a/module/rdpClientCon.c
+++ b/module/rdpClientCon.c
@@ -2684,6 +2684,12 @@ rdpClientConSendPaintRectShmFd(rdpPtr dev, rdpClientCon *clientCon,
size += end_frame_bytes; /* end frame message */
size += 4; /* message 62 data_bytes */
+ if (size > (clientCon->out_s->size - 20))
+ {
+ // Way too big - pre-check will fail
+ LLOGLN(0, ("Size check prob: requested=%d max=%d", size, (clientCon->out_s->size - 20)));
+ }
+
rdpClientConPreCheck(dev, clientCon, size);
s = clientCon->out_s;
out_uint16_le(s, 62);
@@ -2836,10 +2842,17 @@ rdpCapRect(rdpClientCon *clientCon, BoxPtr cap_rect, struct image_data *id)
LLOGLN(10, ("rdpCapRect: cap_rect x1 %d y1 %d x2 %d y2 %d",
cap_rect->x1, cap_rect->y1, cap_rect->x2, cap_rect->y2));
rdpRegionIntersect(cap_dirty, cap_dirty, clientCon->dirtyRegion);
+ num_rects = REGION_NUM_RECTS(cap_dirty);
+ if (num_rects > 2000) // What should this be??
+ {
+ // Collapse down to the bounding rectangle
+ rdpRegionUnionRect(cap_dirty, rdpRegionExtents(cap_dirty));
+ num_rects = REGION_NUM_RECTS(cap_dirty);
+ }
+
/* make a copy of cap_dirty because it may get altered */
cap_dirty_save = rdpRegionCreate(NullBox, 0);
rdpRegionCopy(cap_dirty_save, cap_dirty);
- num_rects = REGION_NUM_RECTS(cap_dirty);
if (num_rects > 0)
{
rects = 0;This also stops the X server crashing. |
|
We can't do much with crects but drect, yea, we can use rdpRegionExtents to simplify it. It's probably easier on the system to encode more pixels than have this complex region. There is a define for this, MAX_CAPTURE_RECTS that we lost with GFX(my fault) that should be put back. |
|
I create #319 for this |
|
It looks like I removed the use of MAX_CAPTURE_RECTS in 83e434e |
|
Reopen until fix in v0.10. |
matt335672
changed the title
|
Merged into v0.10 branch, I will release v0.10.2 in July or mid-August. |
|
I encountered the exact same error with v0.10.1 but in a different scenario: when I grab a window and start to drag it around fast, eventually everything freezes and the same errors appear in the log file. I use XFCE if it matters. Update: I did some testing and it seems it happens only if you disable effects AND enable "Hide content of windows - When moving". |
|
@Mogaba - look for the buffer overrun error mentioned in the first post in this thread. |
Yep, it's the same error:
Does it mean it's probably be fixed by upgrading to v0.10.2? |
Version: xrdp v0.10.0 and xorgxrdp v0.10.1
Steps to reproduce:
xtermas WM for simplicityxeyes -geometry 3840x2160(exactly the same screen size with xrandr reports)I see the following xorgxrdp log. This hang-up doesn't happen v0.9. So it is related to recent GFX change.
I recalled #64 from the log with the following log line. It looks like a buffer overrun. I also recalled #5 and #62.
It looks like a buffer overrun, I tried to increase buffer size to 64k. I don't see any obvious buffer overrun with 64k buffer but Xorg still hangs up.
The text was updated successfully, but these errors were encountered: