Bug#862824: xserver-xorg-core: tigervncserver crash in WriteToClient shortly after VNC client connecs
On 17/05/17 10:11 PM, Rafal wrote:
> Package: xserver-xorg-core
> Version: 2:1.19.3-1ubuntu1
> Severity: important
> Tags: patch
>
> The problem appears in tigervncserver, but I have made some investigation and I
> have found the problem in xorg server. Namely, I have xfce-session run in
> xstartup script of the vnc server. When I'm trying to connect to the VNC
> server, sometimes first client simply hangs. Second client connects
> successfully, but the server crashes after first mouse click. The crash occurs
> in WriteToClient method, when a member of oco variable is accessed (file io.c
> line 764).
>
> I have made some investigation, what series of events leads to the crash. It is
> as follows:
> * Some X client exits when the server has some pending output
> for it. FlushClient procedure invokes
> _XSERVTransClose(oc->trans_conn) (line 927) and returns -1.
> Note that the _XSERVTransClose closes oc->trans_conn->fd file
> descriptor.
> * The VNC connection arrives. The accept() method returns file
> * descriptor
> equal to the closed by _XSERVTransClose(). A new XserverDesktop
> instance is created, XserverDesktop::addClient() is invoked.
> This method invokes SetNotifyFd(). Normally the SetNotifyFd
> invokes ospoll_add() because this is a new connection. But in
> this case some entry is found and it is updated.
> * The VNC connection hangs because notifications about new data
> * on socket
> aren't added successfully. The server attempts to modify
> notification but it should add new one as it is a new file
> descriptor.
> * Further server crash is caused by modifications of data made in
> SetNotifyFd function. The function assumes that a pointer
> returned by ospoll_data is a notify_fd structure but it is, in
> fact, a ClientPtr. The modification causes further crash in
> WriteToClient when the ClientPtr is used.
>
> In my opinion, because the _XSERVTransClose() closes file descriptor,
> corresponding entry in ospoll should be also removed. It means the
> ospoll_remove() function should be always invoked just before call of
> _XSERVTransClose().
These changes might help for this issue:
https://patchwork.freedesktop.org/patch/155681/
https://cgit.freedesktop.org/xorg/xserver/commit/?id=a82971b07035ee9a4e3ed01326e7c1eab34b5a19
https://cgit.freedesktop.org/xorg/xserver/commit/?id=e2f68296ffb8e40035c0ebd949b67d1e2e424e11
https://cgit.freedesktop.org/xorg/xserver/commit/?id=d9e23ea4228575344e3b4c0443cecc5eb75356e4
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Reply to: