openbmc / obmc-ikvm

GNU General Public License v2.0
23 stars 9 forks source link

obmc-ikvm daemon crashes during client close session #5

Closed LeeTroy closed 3 years ago

LeeTroy commented 3 years ago

I was running a stress test to open/close kvm session from web frontend (webui-vue), about obmc-ikvm crashed with .5% possibility buffer overflow when the connection closed.

obmc-ikvm[547]: 25/08/2021 03:29:19 Enabling full-color cursor updates for client 127.0.0.1
obmc-ikvm[547]: 25/08/2021 03:29:19 Using tight encoding for client 127.0.0.1
obmc-ikvm[547]: 25/08/2021 03:29:19 Sending rfbEncodingExtDesktopSize for size (1024x768)
obmc-ikvm[547]: 25/08/2021 03:29:21 rfbSendUpdateBuf: write: Connection reset by peer
obmc-ikvm[547]: *** buffer overflow detected ***: terminated
systemd[1]: start-ipkvm.service: Main process exited, code=killed, status=6/ABRT
systemd[1]: start-ipkvm.service: Failed with result 'signal'.
systemd[1]: start-ipkvm.service: Scheduled restart job, restart counter is at 2.

The following code snip need to check the return value, if it return FALSE https://github.com/openbmc/obmc-ikvm/blob/2d2f3dab4253a3d6edf6bef98c5f880f51d2394b/ikvm_server.cpp#L162

https://github.com/openbmc/obmc-ikvm/blob/2d2f3dab4253a3d6edf6bef98c5f880f51d2394b/ikvm_server.cpp#L175

Reference: https://github.com/LibVNC/libvncserver/blob/2aa20dad4c23c18948d3f63b33f9dfec1f837729/libvncserver/rfbserver.c#L3609-L3628

LeeTroy commented 3 years ago

After investigating, it turns out the obmc-ikvm implementated 2 threads to access libvncserver, however libvncserver doesn't compile with HAVE_PTHREAD turns on. So the client structure cl in libvncserver has no protection across all apis, it might lead to double free (calling ClientClose(cl) twice).

I'll submit a bbappend for libvncserver to enable pthread flag and close this issue after patch got merged.

LeeTroy commented 3 years ago

Submitted: https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/46204

LeeTroy commented 3 years ago

Merged.