GNOME Bugzilla – Bug 635397
rfbsrc: avoid infinite loop if source gets disconnected and don't crash if frame geometry changes
Last modified: 2010-12-12 23:39:34 UTC
Created attachment 174935 [details] [review] rfbsrc patch <mife> 12:51:47> I don't need another bugzilla account so never posted there, but http://airattack-central.com/gst-librfbpatch.patch is a patch I've got working so that librfbsrc (vnc source) doesn't get caught in an infinite loop if the source gets disconnected and also supports gracefully failing upon detecting the frame geometry has increased (rather than segfaulting). This was done so I could use a gstreamer pipeline to record VM sessions from libvirt - I don't think t -------------- Asked for git format-patch formatted patch. -------------- <mife> Th patch fixes two bugs. The first is an infinite loop when it reads from the socket. Although theres sanity checks in the code it appears as though this doesn't always matter. Which ultimately causes a while loop further up the code to keep on executing. You can't get out of the loop without terminating the program externally <mife> I added a disconnected flag to the rfbdecoder struct and set that to true when there is a disconnect. The disconnect code does work its just lots of places in the code doesn't seem to care about what gets returned from that function. <mife> I additionally added a check further up the code for the disconnect, which causes the rfbsrc to die more gracefully. <mife> Im pretty sure that merely patching it for my scenario as I suspect therse more places which can catch into the loop <mife> The second bug I fixed was a geometry one. VMs that utilize VNC connections often dynamically switch geometries in the remote frame buffer. <mife> gstreamers plugin can't handle this as it allocates its memory buffer after determining what the width of its rectangle is going to be when it initialized. This causes a segfault because the code attempts to write past the heap it was allocated when the screen size changes. <__tim> ok <mife> I basically do a check at the top now to determine if the size of the rectangle about to be written into the buffer is greater than the size of the allocated buffer. If true it gracefully errors and disconnects (uses the same disconnect method I put in place) <__tim> and what's your name + e-mail please? (so I can dump all this into bugzilla) <mife> so, now it doesn segfault but exits gracefully. <mife> I guess it would actually be better to reallocate the buffer. <mife> matthew.ife@ukfast.co.uk <mife> Matthew Ife
Without any knowledge about the code it looks sensible to me. Are you going to review/push this?
Ah yes, let's push this. commit 51c63587a16518fa322eb4fcdcc08102c8b0cc3e Author: Matthew Ife <matthew.ife@ukfast.co.uk> Date: Sun Dec 12 23:34:02 2010 +0000 rfbsrc: fail more gracefully if source gets disconnected or geometry changes Don't get caught in an infinite loop if the source gets disconnected and also support gracefully failing upon detecting the frame geometry has increased (rather than segfaulting). https://bugzilla.gnome.org/show_bug.cgi?id=635397 Also added a decoder->disconnected=FALSE in rfb_decoder_connect_tcp(), since the decoder struct is created only in rfbsrc init and not in the start vfunc.