After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 635397 - rfbsrc: avoid infinite loop if source gets disconnected and don't crash if frame geometry changes
rfbsrc: avoid infinite loop if source gets disconnected and don't crash if f...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 0.10.21
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-11-21 00:18 UTC by Tim-Philipp Müller
Modified: 2010-12-12 23:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rfbsrc patch (2.01 KB, patch)
2010-11-21 00:18 UTC, Tim-Philipp Müller
committed Details | Review

Description Tim-Philipp Müller 2010-11-21 00:18:43 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
Comment 1 Sebastian Dröge (slomo) 2010-12-12 15:47:23 UTC
Without any knowledge about the code it looks sensible to me. Are you going to review/push this?
Comment 2 Tim-Philipp Müller 2010-12-12 23:39:24 UTC
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.