Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xfreerdp crashes, maybe due to glyph cache. #1500

Closed
macarthor opened this issue Sep 22, 2013 · 31 comments · Fixed by #1508
Closed

xfreerdp crashes, maybe due to glyph cache. #1500

macarthor opened this issue Sep 22, 2013 · 31 comments · Fixed by #1508
Assignees

Comments

@macarthor
Copy link

ubuntu 12.04.3 x86 desktop fresh install, freerdp master 20130922.

apt-get install libssl-dev libx11-dev libxext-dev libxinerama-dev libxcursor-dev libxdamage-dev libxv-dev libxkbfile-dev libasound2-dev libcups2-dev libxml2 libxml2-dev libxrandr-dev libgstreamer0.10-dev libgstreamer-plugins-base0.10-dev libavutil-dev libavcodec-dev libxi-dev libxtst-dev uuid-dev libudev-dev libusb-1.0-0-dev libdbus-glib-1-dev
vim channels/urbdrc/ChannelOptions.cmake set(OPTION_CLIENT_DEFAULT ON)
cmake . -DCMAKE_INSTALL_PREFIX=/home/freerdp/usr
make && make install

xfreerdp /bpp:16 /f /fonts /compression /cert-ignore /sound:sys:alsa /drive:SharedFolder,/media /u:xxx /p:xxx /v:xxx

A suggestion: review codes, and add "if (NULL != pointer)" before EVERY "free(pointer)", and add "pointer = NULL" after EVERY "free(pointer)". I think this review time is NECESSARY.

(NOTE: /home/freerdp is a symlink to /home/freerdp-5257)

loading channel rdpsnd
connected to xxx.xxx.xxx.xxx:3389
Loading device service drive (static)
*** glibc detected *** /home/freerdp/usr/bin/xfreerdp: double free or corruption (fasttop): 0xb4c52c10 ***
======= Backtrace: =========
/lib/i386-linux-gnu/libc.so.6(+0x75ee2)[0xb75a3ee2]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libfreerdp-cache.so.1.1(glyph_cache_put+0x6e)[0xb730016e]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libfreerdp-cache.so.1.1(update_gdi_fast_glyph+0x103)[0xb73008f3]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libfreerdp-core.so.1.1(update_recv_primary_order+0x1ed)[0xb73768cd]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libfreerdp-core.so.1.1(update_recv_order+0x56)[0xb7377306]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libfreerdp-core.so.1.1(+0x3ee6a)[0xb7385e6a]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libfreerdp-core.so.1.1(fastpath_recv_updates+0xf0)[0xb7386310]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libfreerdp-core.so.1.1(+0x3dbf8)[0xb7384bf8]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libfreerdp-core.so.1.1(transport_check_fds+0xbb)[0xb73887db]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libfreerdp-core.so.1.1(rdp_check_fds+0x1e)[0xb73850ee]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libfreerdp-core.so.1.1(freerdp_check_fds+0x2a)[0xb737787a]
/home/freerdp-5257/usr/bin/../lib/i386-linux-gnu/libxfreerdp-client.so.1.1(xf_thread+0x513)[0xb7752003]
/lib/i386-linux-gnu/libpthread.so.0(+0x6d4c)[0xb725ed4c]
/lib/i386-linux-gnu/libc.so.6(clone+0x5e)[0xb761cdde]
======= Memory map: ========
08048000-0804a000 r-xp 00000000 08:02 234 /home/freerdp-5257/usr/bin/xfreerdp
0804a000-0804b000 r--p 00001000 08:02 234 /home/freerdp-5257/usr/bin/xfreerdp
0804b000-0804c000 rw-p 00002000 08:02 234 /home/freerdp-5257/usr/bin/xfreerdp
0908f000-090fe000 rw-p 00000000 00:00 0 [heap]
b29fe000-b29ff000 ---p 00000000 00:00 0
b29ff000-b31ff000 rw-p 00000000 00:00 0 [stack:5054]
b31ff000-b3200000 ---p 00000000 00:00 0
b3200000-b3a00000 rw-p 00000000 00:00 0 [stack:5053]
b3a00000-b3a21000 rw-p 00000000 00:00 0
b3a21000-b3b00000 ---p 00000000 00:00 0
b3bfe000-b3bff000 ---p 00000000 00:00 0
b3bff000-b43ff000 rw-p 00000000 00:00 0 [stack:5052]
b43ff000-b4400000 ---p 00000000 00:00 0
b4400000-b4c00000 rw-p 00000000 00:00 0 [stack:5051]
b4c00000-b4cba000 rw-p 00000000 00:00 0
b4cba000-b4d00000 ---p 00000000 00:00 0
b4da3000-b4db8000 r--p 00000000 07:00 101580 /usr/share/locale-langpack/zh_CN/LC_MESSAGES/libc.mo
b4db8000-b4db9000 ---p 00000000 00:00 0
b4db9000-b55da000 rw-p 00000000 00:00 0 [stack:5050]
b55da000-b573f000 r--p 001c8000 08:02 260707 /usr/lib/locale/locale-archive
b573f000-b593f000 r--p 00000000 08:02 260707 /usr/lib/locale/locale-archive
b593f000-b5943000 rw-p 00000000 00:00 0
b5943000-b597d000 r-xp 00000000 07:00 17609 /lib/i386-linux-gnu/libpcre.so.3.12.1
b597d000-b597e000 r--p 00039000 07:00 17609 /lib/i386-linux-gnu/libpcre.so.3.12.1
b597e000-b597f000 rw-p 0003a000 07:00 17609 /lib/i386-linux-gnu/libpcre.so.3.12.1
b597f000-b5980000 rw-p 00000000 00:00 0
b5980000-b599c000 r-xp 00000000 07:00 17683 /lib/i386-linux-gnu/libgcc_s.so.1
b599c000-b599d000 r--p 0001b000 07:00 17683 /lib/i386-linux-gnu/libgcc_s.so.1
b599d000-b599e000 rw-p 0001c000 07:00 17683 /lib/i386-linux-gnu/libgcc_s.so.1
b599e000-b5a29000 r-xp 00000000 07:00 68568 /usr/lib/i386-linux-gnu/liborc-0.4.so.0.16.0
b5a29000-b5a2a000 r--p 0008a000 07:00 68568 /usr/lib/i386-linux-gnu/liborc-0.4.so.0.16.0
b5a2a000-b5a2e000 rw-p 0008b000 07:00 68568 /usr/lib/i386-linux-gnu/liborc-0.4.so.0.16.0
b5a2e000-b5a34000 r-xp 00000000 07:00 70042 /usr/lib/i386-linux-gnu/libogg.so.0.7.1
b5a34000-b5a35000 r--p 00005000 07:00 70042 /usr/lib/i386-linux-gnu/libogg.so.0.7.1
b5a35000-b5a36000 rw-p 00006000 07:00 70042 /usr/lib/i386-linux-gnu/libogg.so.0.7.1
b5a36000-b5a87000 r-xp 00000000 07:00 17700 /lib/i386-linux-gnu/libssl.so.1.0.0
b5a87000-b5a89000 r--p 00050000 07:00 17700 /lib/i386-linux-gnu/libssl.so.1.0.0
b5a89000-b5a8d000 rw-p 00052000 07:00 17700 /lib/i386-linux-gnu/libssl.so.1.0.0
b5a8d000-b5a8e000 rw-p 00000000 00:00 0
b5a8e000-b5a93000 r-xp 00000000 07:00 69639 /usr/lib/i386-linux-gnu/libXdmcp.so.6.0.0
b5a93000-b5a94000 r--p 00004000 07:00 69639 /usr/lib/i386-linux-gnu/libXdmcp.so.6.0.0
b5a94000-b5a95000 rw-p 00005000 07:00 69639 /usr/lib/i386-linux-gnu/libXdmcp.so.6.0.0
b5a95000-b5a97000 r-xp 00000000 07:00 69175 /usr/lib/i386-linux-gnu/libXau.so.6.0.0
b5a97000-b5a98000 r--p 00001000 07:00 69175 /usr/lib/i386-linux-gnu/libXau.so.6.0.0
b5a98000-b5a99000 rw-p 00002000 07:00 69175 /usr/lib/i386-linux-gnu/libXau.so.6.0.0
b5a99000-b5af7000 r-xp 00000000 07:00 68656 /usr/lib/i386-linux-gnu/libgstbase-0.10.so.0.30.0
b5af7000-b5af8000 r--p 0005d000 07:00 68656 /usr/lib/i386-linux-gnu/libgstbase-0.10.so.0.30.0
b5af8000-b5af9000 rw-p 0005e000 07:00 68656 /usr/lib/i386-linux-gnu/libgstbase-0.10.so.0.30.0
b5af9000-b5afe000 r-xp 00000000 07:00 69865 /usr/lib/i386-linux-gnu/libffi.so.6.0.0
b5afe000-b5aff000 r--p 00004000 07:00 69865 /usr/lib/i386-linux-gnu/libffi.so.6.0.0
b5aff000-b5b00000 rw-p 00005000 07:00 69865 /usr/lib/i386-linux-gnu/libffi.so.6.0.0
b5b00000-b5b01000 rw-p 00000000 00:00 0
b5b01000-b5c48000 r-xp 00000000 07:00 69143 /usr/lib/i386-linux-gnu/libxml2.so.2.7.8
b5c48000-b5c4c000 r--p 00147000 07:00 69143 /usr/lib/i386-linux-gnu/libxml2.so.2.7.8
b5c4c000-b5c4d000 rw-p 0014b000 07:00 69143 /usr/lib/i386-linux-gnu/libxml2.so.2.7.8
b5c4d000-b5c4e000 rw-p 00000000 00:00 0
b5c4e000-b5d45000 r-xp 00000000 07:00 17779 /lib/i386-linux-gnu/libglib-2.0.so.0.3200.3
b5d45000-b5d46000 r--p 000f6000 07:00 17779 /lib/i386-linux-gnu/libglib-2.0.so.0.3200.3
b5d46000-b5d47000 rw-p 000f7000 07:00 17779 /lib/i386-linux-gnu/libglib-2.0.so.0.3200.3
b5d47000-b5d4a000 r-xp 00000000 07:00 69770 /usr/lib/i386-linux-gnu/libgmodule-2.0.so.0.3200.3
b5d4a000-b5d4b000 r--p 00002000 07:00 69770 /usr/lib/i386-linux-gnu/libgmodule-2.0.so.0.3200.3
b5d4b000-b5d4c000 rw-p 00003000 07:00 69770 /usr/lib/i386-linux-gnu/libgmodule-2.0.so.0.3200.3
b5d4c000-b5d60000 r-xp 00000000 07:00 17710 /lib/i386-linux-gnu/libz.so.1.2.3.4
b5d60000-b5d61000 r--p 00013000 07:00 17710 /lib/i386-linux-gnu/libz.so.1.2.3.4
b5d61000-b5d62000 rw-p 00014000 07:00 17710 /lib/i386-linux-gnu/libz.so.1.2.3.4
b5d62000-b5d72000 r-xp 00000000 08:02 263746 /usr/lib/i386-linux-gnu/libva.so.1.3200.0
b5d72000-b5d73000 r--p 0000f000 08:02 263746 /usr/lib/i386-linux-gnu/libva.so.1.3200.0
b5d73000-b5d74000 rw-p 00010000 08:02 263746 /usr/lib/i386-linux-gnu/libva.so.1.3200.0
b5d74000-b5d79000 rw-p 00000000 00:00 0
b5d79000-b5d85000 r-xp 00000000 08:02 261692 /usr/lib/libgsm.so.1.0.12
b5d85000-b5d86000 r--p 0000b000 08:02 261692 /usr/lib/libgsm.so.1.0.12
b5d86000-b5d87000 rw-p 0000c000 08:02 261692 /usr/lib/libgsm.so.1.0.12
b5d87000-b5e42000 r-xp 00000000 08:02 262181 /usr/lib/libschroedinger-1.0.so.0.11.0
b5e42000-b5e44000 r--p 000ba000 08:02 262181 /usr/lib/libschroedinger-1.0.so.0.11.0
b5e44000-b5e45000 rw-p 000bc000 08:02 262181 /usr/lib/libschroedinger-1.0.so.0.11.0
b5e45000-b5e64000 r-xp 00000000 07:00 69928 /usr/lib/i386-linux-gnu/sse2/libspeex.so.1.5.0
b5e64000-b5e65000 r--p 0001e000 07:00 69928 /usr/lib/i386-linux-gnu/sse2/libspeex.so.1.5.0
b5e65000-b5e66000 rw-p 0001f000 07:00 69928 /usr/lib/i386-linux-gnu/sse2/libspeex.so.1.5.0
b5e66000-b5e81000 r-xp 00000000 07:00 68735 /usr/lib/i386-linux-gnu/libtheoradec.so.1.1.4
b5e81000-b5e82000 r--p 0001a000 07:00 68735 /usr/lib/i386-linux-gnu/libtheoradec.so.1.1.4
b5e82000-b5e83000 rw-p 0001b000 07:00 68735 /usr/lib/i386-linux-gnu/libtheoradec.so.1.1.4
b5e83000-b5ec2000 r-xp 00000000 07:00 68513 /usr/lib/i386-linux-gnu/libtheoraenc.so.1.1.2
b5ec2000-b5ec3000 r--p 0003f000 07:00 68513 /usr/lib/i386-linux-gnu/libtheoraenc.so.1.1.2
b5ec3000-b5ec4000 rw-p 00040000 07:00 68513 /usr/lib/i386-linux-gnu/libtheoraenc.so.1.1.2
b5ec4000-b5ec5000 rw-p 00000000 00:00 0
b5ec5000-b5eee000 r-xp 00000000 07:00 69250 /usr/lib/i386-linux-gnu/libvorbis.so.0.4.5
b5eee000-b5eef000 r--p 00028000 07:00 69250 /usr/lib/i386-linux-gnu/libvorbis.so.0.4.5
b5eef000-b5ef0000 rw-p 00029000 07:00 69250 /usr/lib/i386-linux-gnu/libvorbis.so.0.4.5
b5ef0000-b6056000 r-xp 00000000 07:00 68734 /usr/lib/i386-linux-gnu/libvorbisenc.so.2.0.8
b6056000-b6067000 r--p 00165000 07:00 68734 /usr/lib/i386-linux-gnu/libvorbisenc.so.2.0.8
b6067000-b6068000 rw-p 00176000 07:00 68734 /usr/lib/i386-linux-gnu/libvorbisenc.so.2.0.8
b6068000-b6104000 r-xp 00000000 08:02 263748 /usr/lib/libvpx.so.1.0.0
b6104000-b6105000 r--p 0009c000 08:02 263748 /usr/lib/libvpx.so.1.0.0
b6105000-b6106000 rw-p 0009d000 08:02 263748 /usr/lib/libvpx.so.1.0.0
b6106000-b6110000 rw-p 00000000 00:00 0
b6110000-b6112000 r-xp 00000000 08:02 38 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-environment.so.0.1.0
b6112000-b6113000 r--p 00001000 08:02 38 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-environment.so.0.1.0
b6113000-b6114000 rw-p 00002000 08:02 38 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-environment.so.0.1.0
b6114000-b6115000 r-xp 00000000 08:02 56 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-heap.so.0.1.0
b6115000-b6116000 r--p 00000000 08:02 56 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-heap.so.0.1.0
b6116000-b6117000 rw-p 00001000 08:02 56 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-heap.so.0.1.0
b6117000-b6118000 rw-p 00000000 00:00 0
b6118000-b611b000 r-xp 00000000 08:02 19 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-pool.so.0.1.0
b611b000-b611c000 r--p 00002000 08:02 19 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-pool.so.0.1.0
b611c000-b611d000 rw-p 00003000 08:02 19 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-pool.so.0.1.0
b611d000-b6123000 r-xp 00000000 08:02 28 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-primitives.so.1.1.0
b6123000-b6124000 r--p 00005000 08:02 28 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-primitives.so.1.1.0
b6124000-b6125000 rw-p 00006000 08:02 28 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-primitives.so.1.1.0
b6125000-b6128000 r-xp 00000000 08:02 59 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-input.so.0.1.0
b6128000-b6129000 r--p 00002000 08:02 59 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-input.so.0.1.0
b6129000-b612a000 rw-p 00003000 08:02 59 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-input.so.0.1.0
b612a000-b614d000 r-xp 00000000 07:00 68768 /usr/lib/i386-linux-gnu/libxkbfile.so.1.0.2
b614d000-b614e000 ---p 00023000 07:00 68768 /usr/lib/i386-linux-gnu/libxkbfile.so.1.0.2
b614e000-b614f000 r--p 00023000 07:00 68768 /usr/lib/i386-linux-gnu/libxkbfile.so.1.0.2
b614f000-b6150000 rw-p 00024000 07:00 68768 /usr/lib/i386-linux-gnu/libxkbfile.so.1.0.2
b6150000-b6151000 rw-p 00000000 00:00 0
b6151000-b6152000 r-xp 00000000 08:02 57 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-library.so.0.1.0
b6152000-b6153000 r--p 00000000 08:02 57 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-library.so.0.1.0
b6153000-b6154000 rw-p 00001000 08:02 57 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-library.so.0.1.0
b6154000-b615d000 r-xp 00000000 08:02 37 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-rpc.so.0.1.0
b615d000-b615e000 r--p 00008000 08:02 37 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-rpc.so.0.1.0
b615e000-b615f000 rw-p 00009000 08:02 37 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-rpc.so.0.1.0
b615f000-b6170000 r-xp 00000000 08:02 27 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-sspi.so.0.1.0
b6170000-b6171000 r--p 00010000 08:02 27 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-sspi.so.0.1.0
b6171000-b6172000 rw-p 00011000 08:02 27 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-sspi.so.0.1.0
b6172000-b6173000 r-xp 00000000 08:02 51 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-dsparse.so.0.1.0
b6173000-b6174000 r--p 00000000 08:02 51 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-dsparse.so.0.1.0
b6174000-b6175000 rw-p 00001000 08:02 51 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-dsparse.so.0.1.0
b6175000-b6176000 rw-p 00000000 00:00 0
b6176000-b6179000 r-xp 00000000 08:02 31 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-registry.so.0.1.0
b6179000-b617a000 r--p 00002000 08:02 31 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-registry.so.0.1.0
b617a000-b617b000 rw-p 00003000 08:02 31 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-registry.so.0.1.0
b617b000-b6188000 r-xp 00000000 08:02 42 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-crypto.so.1.1.0
b6188000-b6189000 r--p 0000c000 08:02 42 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-crypto.so.1.1.0
b6189000-b618a000 rw-p 0000d000 08:02 42 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-crypto.so.1.1.0
b618a000-b631c000 r-xp 00000000 07:00 17675 /lib/i386-linux-gnu/libcrypto.so.1.0.0
b631c000-b632b000 r--p 00192000 07:00 17675 /lib/i386-linux-gnu/libcrypto.so.1.0.0
b632b000-b6332000 rw-p 001a1000 07:00 17675 /lib/i386-linux-gnu/libcrypto.so.1.0.0
b6332000-b6335000 rw-p 00000000 00:00 0
b6335000-b6339000 r-xp 00000000 07:00 69854 /usr/lib/i386-linux-gnu/libXfixes.so.3.1.0
b6339000-b633a000 r--p 00004000 07:00 69854 /usr/lib/i386-linux-gnu/libXfixes.so.3.1.0
b633a000-b633b000 rw-p 00005000 07:00 69854 /usr/lib/i386-linux-gnu/libXfixes.so.3.1.0
b633b000-b633e000 r-xp 00000000 07:00 17610 /lib/i386-linux-gnu/libdl-2.15.so
b633e000-b633f000 r--p 00002000 07:00 17610 /lib/i386-linux-gnu/libdl-2.15.so
b633f000-b6340000 rw-p 00003000 07:00 17610 /lib/i386-linux-gnu/libdl-2.15.so
b6340000-b6341000 rw-p 00000000 00:00 0
b6341000-b6360000 r-xp 00000000 07:00 69823 /usr/lib/i386-linux-gnu/libxcb.so.1.1.0
b6360000-b6361000 r--p 0001f000 07:00 69823 /usr/lib/i386-linux-gnu/libxcb.so.1.1.0
b6361000-b6362000 rw-p 00020000 07:00 69823 /usr/lib/i386-linux-gnu/libxcb.so.1.1.0
b6362000-b6369000 r-xp 00000000 07:00 17672 /lib/i386-linux-gnu/librt-2.15.so
b6369000-b636a000 r--p 00006000 07:00 17672 /lib/i386-linux-gnu/librt-2.15.so
b636a000-b636b000 rw-p 00007000 07:00 17672 /lib/i386-linux-gnu/librt-2.15.so
b636b000-b636d000 r-xp 00000000 08:02 63 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-sysinfo.so.0.1.0
b636d000-b636e000 r--p 00001000 08:02 63 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-sysinfo.so.0.1.0
b636e000-b636f000 rw-p 00002000 08:02 63 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-sysinfo.so.0.1.0
b636f000-b6375000 r-xp 00000000 08:02 55 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-common.so.1.1.0-beta1
b6375000-b6376000 r--p 00005000 08:02 55 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-common.so.1.1.0-beta1
b6376000-b6377000 rw-p 00006000 08:02 55 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-common.so.1.1.0-beta1
b6377000-b6378000 rw-p 00000000 00:00 0
b6378000-b6465000 r-xp 00000000 07:00 68776 /usr/lib/i386-linux-gnu/libasound.so.2.0.0
b6465000-b6469000 r--p 000ec000 07:00 68776 /usr/lib/i386-linux-gnu/libasound.so.2.0.0
b6469000-b646a000 rw-p 000f0000 07:00 68776 /usr/lib/i386-linux-gnu/libasound.so.2.0.0
b646a000-b6471000 r-xp 00000000 07:00 68539 /usr/lib/i386-linux-gnu/libXrandr.so.2.2.0
b6471000-b6472000 r--p 00006000 07:00 68539 /usr/lib/i386-linux-gnu/libXrandr.so.2.2.0
b6472000-b6473000 rw-p 00007000 07:00 68539 /usr/lib/i386-linux-gnu/libXrandr.so.2.2.0
b6473000-b6482000 r-xp 00000000 07:00 68378 /usr/lib/i386-linux-gnu/libgstinterfaces-0.10.so.0.25.0
b6482000-b6483000 r--p 0000f000 07:00 68378 /usr/lib/i386-linux-gnu/libgstinterfaces-0.10.so.0.25.0
b6483000-b6484000 rw-p 00010000 07:00 68378 /usr/lib/i386-linux-gnu/libgstinterfaces-0.10.so.0.25.0
b6484000-b648f000 r-xp 00000000 07:00 68517 /usr/lib/i386-linux-gnu/libgstapp-0.10.so.0.25.0
b648f000-b6490000 r--p 0000a000 07:00 68517 /usr/lib/i386-linux-gnu/libgstapp-0.10.so.0.25.0
b6490000-b6491000 rw-p 0000b000 07:00 68517 /usr/lib/i386-linux-gnu/libgstapp-0.10.so.0.25.0
b6491000-b64de000 r-xp 00000000 07:00 69810 /usr/lib/i386-linux-gnu/libgobject-2.0.so.0.3200.3
b64de000-b64df000 r--p 0004c000 07:00 69810 /usr/lib/i386-linux-gnu/libgobject-2.0.so.0.3200.3
b64df000-b64e0000 rw-p 0004d000 07:00 69810 /usr/lib/i386-linux-gnu/libgobject-2.0.so.0.3200.3
b64e0000-b64e1000 rw-p 00000000 00:00 0
b64e1000-b65c4000 r-xp 00000000 07:00 70026 /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0.30.0
b65c4000-b65c7000 r--p 000e2000 07:00 70026 /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0.30.0
b65c7000-b65c8000 rw-p 000e5000 07:00 70026 /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0.30.0
b65c8000-b65c9000 rw-p 00000000 00:00 0
b65c9000-b65e6000 r-xp 00000000 08:02 261198 /usr/lib/i386-linux-gnu/i686/cmov/libavutil.so.51.22.1
b65e6000-b65e7000 r--p 0001c000 08:02 261198 /usr/lib/i386-linux-gnu/i686/cmov/libavutil.so.51.22.1
b65e7000-b65e8000 rw-p 0001d000 08:02 261198 /usr/lib/i386-linux-gnu/i686/cmov/libavutil.so.51.22.1
b65e8000-b65eb000 rw-p 00000000 00:00 0
b65eb000-b6cca000 r-xp 00000000 08:02 261142 /usr/lib/i386-linux-gnu/i686/cmov/libavcodec.so.53.35.0
b6cca000-b6ccb000 r--p 006de000 08:02 261142 /usr/lib/i386-linux-gnu/i686/cmov/libavcodec.so.53.35.0
b6ccb000-b6cda000 rw-p 006df000 08:02 261142 /usr/lib/i386-linux-gnu/i686/cmov/libavcodec.so.53.35.0
b6cda000-b7248000 rw-p 00000000 00:00 0
b7248000-b7249000 r-xp 00000000 08:02 50 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-interlocked.so.0.1.0
b7249000-b724a000 r--p 00001000 08:02 50 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-interlocked.so.0.1.0
b724a000-b724b000 rw-p 00002000 08:02 50 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-interlocked.so.0.1.0
b724b000-b724e000 r-xp 00000000 08:02 20 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-file.so.0.1.0
b724e000-b724f000 r--p 00002000 08:02 20 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-file.so.0.1.0
b724f000-b7250000 rw-p 00003000 08:02 20 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-file.so.0.1.0
b7250000-b7251000 rw-p 00000000 00:00 0
b7251000-b7256000 r-xp 00000000 08:02 45 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-path.so.0.1.0
b7256000-b7257000 r--p 00004000 08:02 45 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-path.so.0.1.0
b7257000-b7258000 rw-p 00005000 08:02 45 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-path.so.0.1.0
b7258000-b726f000 r-xp 00000000 07:00 17641 /lib/i386-linux-gnu/libpthread-2.15.so
b726f000-b7270000 r--p 00016000 07:00 17641 /lib/i386-linux-gnu/libpthread-2.15.so
b7270000-b7271000 rw-p 00017000 07:00 17641 /lib/i386-linux-gnu/libpthread-2.15.so
b7271000-b7273000 rw-p 00000000 00:00 0
b7273000-b7274000 r-xp 00000000 08:02 62 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-handle.so.0.1.0
b7274000-b7275000 r--p 00000000 08:02 62 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-handle.so.0.1.0
b7275000-b7276000 rw-p 00001000 08:02 62 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-handle.so.0.1.0
b7276000-b72a0000 r-xp 00000000 07:00 17657 /lib/i386-linux-gnu/libm-2.15.so
b72a0000-b72a1000 r--p 00029000 07:00 17657 /lib/i386-linux-gnu/libm-2.15.so
b72a1000-b72a2000 rw-p 0002a000 07:00 17657 /lib/i386-linux-gnu/libm-2.15.so
b72a2000-b72a6000 r-xp 00000000 08:02 54 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-crt.so.0.1.0
b72a6000-b72a7000 r--p 00003000 08:02 54 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-crt.so.0.1.0
b72a7000-b72a8000 rw-p 00004000 08:02 54 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-crt.so.0.1.0
b72a8000-b72a9000 rw-p 00000000 00:00 0
b72a9000-b72c5000 r-xp 00000000 08:02 60 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-utils.so.0.1.0
b72c5000-b72c6000 r--p 0001b000 08:02 60 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-utils.so.0.1.0
b72c6000-b72c7000 rw-p 0001c000 08:02 60 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-utils.so.0.1.0
b72c7000-b72d7000 rw-p 00000000 00:00 0
b72d7000-b72fa000 r-xp 00000000 08:02 41 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-codec.so.1.1.0
b72fa000-b72fb000 r--p 00022000 08:02 41 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-codec.so.1.1.0
b72fb000-b72fc000 rw-p 00023000 08:02 41 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-codec.so.1.1.0
b72fc000-b7303000 r-xp 00000000 08:02 24 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-cache.so.1.1.0
b7303000-b7304000 r--p 00006000 08:02 24 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-cache.so.1.1.0
b7304000-b7305000 rw-p 00007000 08:02 24 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-cache.so.1.1.0
b7305000-b730d000 r-xp 00000000 08:02 33 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-utils.so.1.1.0
b730d000-b730e000 r--p 00007000 08:02 33 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-utils.so.1.1.0
b730e000-b730f000 rw-p 00008000 08:02 33 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-utils.so.1.1.0
b730f000-b7312000 r-xp 00000000 08:02 21 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-rail.so.1.1.0
b7312000-b7313000 r--p 00002000 08:02 21 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-rail.so.1.1.0
b7313000-b7314000 rw-p 00003000 08:02 21 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-rail.so.1.1.0
b7314000-b7315000 rw-p 00000000 00:00 0
b7315000-b7329000 r-xp 00000000 08:02 44 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-locale.so.1.1.0
b7329000-b732d000 r--p 00013000 08:02 44 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-locale.so.1.1.0
b732d000-b732e000 rw-p 00017000 08:02 44 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-locale.so.1.1.0
b732e000-b732f000 rw-p 00000000 00:00 0
b732f000-b7345000 r-xp 00000000 08:02 35 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-gdi.so.1.1.0
b7345000-b7346000 r--p 00015000 08:02 35 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-gdi.so.1.1.0
b7346000-b7347000 rw-p 00016000 08:02 35 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-gdi.so.1.1.0
b7347000-b73b2000 r-xp 00000000 08:02 52 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-core.so.1.1.0
b73b2000-b73b3000 ---p 0006b000 08:02 52 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-core.so.1.1.0
b73b3000-b73b4000 r--p 0006b000 08:02 52 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-core.so.1.1.0
b73b4000-b73b7000 rw-p 0006c000 08:02 52 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-core.so.1.1.0
b73b7000-b73bf000 r-xp 00000000 07:00 69801 /usr/lib/i386-linux-gnu/libXrender.so.1.3.0
b73bf000-b73c0000 r--p 00007000 07:00 69801 /usr/lib/i386-linux-gnu/libXrender.so.1.3.0
b73c0000-b73c1000 rw-p 00008000 07:00 69801 /usr/lib/i386-linux-gnu/libXrender.so.1.3.0
b73c1000-b73cf000 r-xp 00000000 07:00 68665 /usr/lib/i386-linux-gnu/libXi.so.6.1.0
b73cf000-b73d0000 r--p 0000d000 07:00 68665 /usr/lib/i386-linux-gnu/libXi.so.6.1.0
b73d0000-b73d1000 rw-p 0000e000 07:00 68665 /usr/lib/i386-linux-gnu/libXi.so.6.1.0
b73d1000-b73d2000 rw-p 00000000 00:00 0
b73d2000-b73d6000 r-xp 00000000 07:00 68663 /usr/lib/i386-linux-gnu/libXv.so.1.0.0
b73d6000-b73d7000 r--p 00003000 07:00 68663 /usr/lib/i386-linux-gnu/libXv.so.1.0.0
b73d7000-b73d8000 rw-p 00004000 07:00 68663 /usr/lib/i386-linux-gnu/libXv.so.1.0.0
b73d8000-b73e1000 r-xp 00000000 07:00 69805 /usr/lib/i386-linux-gnu/libXcursor.so.1.0.2
b73e1000-b73e2000 r--p 00008000 07:00 69805 /usr/lib/i386-linux-gnu/libXcursor.so.1.0.2
b73e2000-b73e3000 rw-p 00009000 07:00 69805 /usr/lib/i386-linux-gnu/libXcursor.so.1.0.2
b73e3000-b73f3000 r-xp 00000000 07:00 68837 /usr/lib/i386-linux-gnu/libXext.so.6.4.0
b73f3000-b73f4000 r--p 0000f000 07:00 68837 /usr/lib/i386-linux-gnu/libXext.so.6.4.0
b73f4000-b73f5000 rw-p 00010000 07:00 68837 /usr/lib/i386-linux-gnu/libXext.so.6.4.0
b73f5000-b73f7000 r-xp 00000000 07:00 69912 /usr/lib/i386-linux-gnu/libXinerama.so.1.0.0
b73f7000-b73f8000 r--p 00001000 07:00 69912 /usr/lib/i386-linux-gnu/libXinerama.so.1.0.0
b73f8000-b73f9000 rw-p 00002000 07:00 69912 /usr/lib/i386-linux-gnu/libXinerama.so.1.0.0
b73f9000-b7529000 r-xp 00000000 08:02 263756 /usr/lib/i386-linux-gnu/libX11.so.6.3.0
b7529000-b752a000 r--p 0012f000 08:02 263756 /usr/lib/i386-linux-gnu/libX11.so.6.3.0
b752a000-b752c000 rw-p 00130000 08:02 263756 /usr/lib/i386-linux-gnu/libX11.so.6.3.0
b752c000-b752e000 rw-p 00000000 00:00 0
b752e000-b76d1000 r-xp 00000000 07:00 17720 /lib/i386-linux-gnu/libc-2.15.so
b76d1000-b76d3000 r--p 001a3000 07:00 17720 /lib/i386-linux-gnu/libc-2.15.so
b76d3000-b76d4000 rw-p 001a5000 07:00 17720 /lib/i386-linux-gnu/libc-2.15.so
b76d4000-b76d7000 rw-p 00000000 00:00 0
b76e1000-b76e2000 rw-p 00000000 00:00 0
b76e2000-b76e3000 rw-s 00000000 00:04 131075 /SYSV00001e61 (deleted)
b76e3000-b76ea000 r--s 00000000 07:00 69376 /usr/lib/i386-linux-gnu/gconv/gconv-modules.cache
b76ea000-b76ec000 r-xp 00000000 08:02 29 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-thread.so.0.1.0
b76ec000-b76ed000 r--p 00001000 08:02 29 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-thread.so.0.1.0
b76ed000-b76ee000 rw-p 00002000 08:02 29 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-thread.so.0.1.0
b76ee000-b76f3000 r-xp 00000000 08:02 18 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-synch.so.0.1.0
b76f3000-b76f4000 r--p 00004000 08:02 18 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-synch.so.0.1.0
b76f4000-b76f5000 rw-p 00005000 08:02 18 /home/freerdp-5257/usr/lib/i386-linux-gnu/libwinpr-synch.so.0.1.0
b76f5000-b7736000 r-xp 00000000 08:02 30 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-client.so.1.1.0
b7736000-b7737000 r--p 00040000 08:02 30 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-client.so.1.1.0
b7737000-b7739000 rw-p 00041000 08:02 30 /home/freerdp-5257/usr/lib/i386-linux-gnu/libfreerdp-client.so.1.1.0
b7739000-b773a000 rw-p 00000000 00:00 0
b773a000-b7757000 r-xp 00000000 08:02 34 /home/freerdp-5257/usr/lib/i386-linux-gnu/libxfreerdp-client.so.1.1.0
b7757000-b7758000 r--p 0001c000 08:02 34 /home/freerdp-5257/usr/lib/i386-linux-gnu/libxfreerdp-client.so.1.1.0
b7758000-b7799000 rw-p 0001d000 08:02 34 /home/freerdp-5257/usr/lib/i386-linux-gnu/libxfreerdp-client.so.1.1.0
b7799000-b779b000 rw-p 00000000 00:00 0
b779b000-b779c000 r-xp 00000000 00:00 0 [vdso]
b779c000-b77bc000 r-xp 00000000 07:00 17617 /lib/i386-linux-gnu/ld-2.15.so
b77bc000-b77bd000 r--p 0001f000 07:00 17617 /lib/i386-linux-gnu/ld-2.15.so
b77bd000-b77be000 rw-p 00020000 07:00 17617 /lib/i386-linux-gnu/ld-2.15.so
bfe27000-bfe48000 rw-p 00000000 00:00 0 [stack]

@macarthor
Copy link
Author

as to my suggestion above, you may only need a function or macro. let's say macro:

#define FREERDP_FREE(pointer) do{if(NULL != pointer){free(pointer);pointer=NULL;}}while(0)

and replace every "free" with "FREERDP_FREE". I think this is the most efficient way.

Though this takes some time, but i think it MUST BE worth.

@macarthor
Copy link
Author

what's more, review codes with "realloc" because it free memory either.

@hardening
Copy link
Contributor

@macarthor i disagree, free() should be called only when we're sure of what we're doing. the if() statement should be added only if we're expecting the pointer to be NULL or valued. All other cases are just unexpected calls that should be managed at a higher level. Adding the if() could hide subtle leaks or bugs.

Could you run xfreerdp with valgrind so that we have some clues of which piece of code freed the memory first ? Thanks.

@macarthor
Copy link
Author

@hardening I use gdb to backtrace the core dump file, and below is the output:

(gdb) bt
#0 0xb772e424 in __kernel_vsyscall ()
#1 0xb74d91df in __GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2 0xb726e481 in fatal_handler (signum=6) at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/utils/signal.c:64
#3
#4 0xb772e424 in __kernel_vsyscall ()
#5 0xb74d91df in __GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#6 0xb74dc825 in __GI_abort () at abort.c:91
#7 0xb751639a in __libc_message (do_abort=2, fmt=0xb7610aa8 "*** glibc detected *** %s: %s: 0x%s ***\n")
at ../sysdeps/unix/sysv/linux/libc_fatal.c:201
#8 0xb7520ee2 in malloc_printerr (action=, str=, ptr=0xb4b52070) at malloc.c:5018
#9 0xb72664ae in glyph_cache_put (glyph_cache=0xb4b0c528, id=6, index=15, glyph=0xb4b4d1a0)
at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/cache/glyph.c:407
#10 0xb7266007 in update_gdi_fast_glyph (context=0x9a55c00, fast_glyph=0x9a6db2c)
at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/cache/glyph.c:294
#11 0xb72e78d2 in update_recv_primary_order (update=0x9a6b560, s=0xb4b52718, flags=133 '\205')
at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/orders.c:3180
#12 0xb72e8461 in update_recv_order (update=0x9a6b560, s=0xb4b52718)
at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/orders.c:3428
#13 0xb72fc47c in fastpath_recv_orders (fastpath=0x9a722f8, s=0xb4b52718)
at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/fastpath.c:158
#14 0xb72fc672 in fastpath_recv_update (fastpath=0x9a722f8, updateCode=0 '\000', size=14227, s=0xb4b52718)
at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/fastpath.c:218
#15 0xb72fcc54 in fastpath_recv_update_data (fastpath=0x9a722f8, s=0xb4b3fbd8)
at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/fastpath.c:362
#16 0xb72fd0e8 in fastpath_recv_updates (fastpath=0x9a722f8, s=0xb4b3fbd8)
at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/fastpath.c:470
#17 0xb72fb164 in rdp_recv_fastpath_pdu (rdp=0x9a58a08, s=0xb4b3fbd8)
at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/rdp.c:897
#18 0xb72fb1b3 in rdp_recv_pdu (rdp=0x9a58a08, s=0xb4b3fbd8) at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/rdp.c:905
#19 0xb72fb302 in rdp_recv_callback (transport=0x9a66c30, s=0xb4b3fbd8, extra=0x9a58a08)
at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/rdp.c:948
#20 0xb72ffce9 in transport_check_fds (ptransport=0x9a58a34) at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/transport.c:749
#21 0xb72fb3ef in rdp_check_fds (rdp=0x9a58a08) at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/rdp.c:979
#22 0xb72e89e1 in freerdp_check_fds (instance=0x9a55950) at /root/freerdp/20130922/FreeRDP-master_debug/libfreerdp/core/freerdp.c:212
#23 0xb76e42cc in xf_thread (param=0x9a55950) at /root/freerdp/20130922/FreeRDP-master_debug/client/X11/xf_client.c:1509
#24 0xb71a7d4c in start_thread (arg=0xb54f7b40) at pthread_create.c:308
#25 0xb7599dde in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:130

glyph.c:407 is free(prevGlyph->aj), and I think prevGlyph->aj is already freed.

I still think that macro is useful to avoid program crash. if you wannna check bug, you may add a printf or something else to the else clause of the if clause.

@macarthor
Copy link
Author

@hardening freerdp uses "if (NULL != prevGlyph->aj)" before "free(prevGlyph->aj)", but does not make prevGlyph->aj NULL after free. so the if null clause is meaningless. it's only meaning when prevGlyph->aj is set NULL after free. C/C++ has no garbage collection, so the pointer will not be set to NULL after freed.

@macarthor
Copy link
Author

@hardening @awakecoding @bmiklautz I reviewed glyph.c history, some uses if NULL clause before free pointer but without setting NULL, some setting NULL pointer but does not check NULL before free.

I think the coding style should be unified, as may avoid errors. I noticed there are many fixes related to memory leak and double free, those would lead to more implicit error without good coding style.

@bmiklautz
Copy link
Member

@macarthor which windows version did you do your test against?

@macarthor
Copy link
Author

@bmiklautz windows 7 pro with sp1 en_us x86. this double free appears almost every time i open TortoiseSvn (latest version, i forget the exact version) GUI.

@bmiklautz
Copy link
Member

@macarthor just installed win7 pro (x86, sp1) and the latest TortoiseSvn version but had no lock to reproduce the problem :(.

@awakecoding
Copy link
Member

@macarthor I strongly disagree on the suggested macro that you'd like to add everywhere. We're often checking for NULL before freeing a pointer, but in reality free() is supposed to do nothing when freeing a NULL pointer: http://stackoverflow.com/questions/2354658/freeing-a-null-pointer

Also, we don't need to set the pointer itself to NULL after freeing it unless it's a pointer we are reusing. This does mean that we need to be careful in all cases where we do reuse memory. This requires manual inspection and understanding of what we do, and there's no magic that would prevent mistakes. Also, you macro would simply not work for cases where you free a pointer to a struct which contains elements that you also need to free.

@macarthor
Copy link
Author

@bmiklautz I know RPD is hard to reproduce the same environment because it strongly depend on the user environment including hardware, software and operation custom...

@macarthor
Copy link
Author

@awakecoding yes, free NULL pointer is not a mistake. But i believe cheking NULL before free is only meaningful when the pointer is set to NULL after free, that is , "if NULL==p" must has pair of "p=NULL".

as to reuse freed pointer, referring to either a freed pointer or a NULL pointer has the same result: segfault.

freeing a struct always needs a function that frees all dynamically allocated members. that macro is responsible to only make sure free() works.

@macarthor
Copy link
Author

@bmiklautz I use CollabNetSubversion-client-1.8.3-1-Win32.exe and TortoiseSVN-1.8.2.24708-win32-svn-1.8.3.msi. Every time I right click a subversion controlled folder and select "SVN update" or "SVN commit...", xfreersh crashes with the error in the first post.

@macarthor
Copy link
Author

I have a workaround patch for this (stable 1.1 branch, 20130924):

glyph.c, about line 407:
if (prevGlyph != NULL)
{
Glyph_Free(glyph_cache->context, prevGlyph);
if (NULL != prevGlyph->aj)
{
free(prevGlyph->aj);
prevGlyph->aj = NULL;
}
free(prevGlyph);
prevGlyph = NULL;
}

about file end:
if (glyph != NULL)
{
Glyph_Free(glyph_cache->context, glyph);
if (NULL != glyph->aj)
{
free(glyph->aj);
glyph->aj = NULL;
}
free(glyph);
glyph = NULL;
glyph_cache->glyphCache[i].entries[j] = NULL;
}

but it does not work always. perhaps glyph->aj is uninitialized when free? perhaps it is pointed to a freed pointer in another struct? i have no idea...

@bmiklautz
Copy link
Member

@macarthor I'll backport the fixes from master to stable at some point - maybe you can retest then.

@macarthor
Copy link
Author

@bmiklautz I test 20130909 0916 0917 0922 0924, some master some stable. all has this issue...

@macarthor
Copy link
Author

@bmiklautz more clues: I use 20130924 stable 1.1 branch. If I use Windows Classic theme, every time I open tortoise svn, xfreerdp crashes. If I use Aero theme such as Landscape (no /aero argument nor /rfx), all works well.

@bmiklautz
Copy link
Member

@macarthor thanks for investigating. If possible please use commit ids as reference since there might be multiple commits per day ;).
So if you use classic theme it crashes and if you use aero it works?
Still same command line as in the beginning?

@macarthor
Copy link
Author

@bmiklautz sorry i don't know how to quote your comment.

  1. I get FreeRdp-master.zip about 2013 09 24 09:55 +8 GMT, but I don't know the exact commit id. Does it exist in the zip?
  2. yes and yes. I've tested 10+ times of opening tortoise svn GUI in both themes.
  3. yes. I call xfreerdp in another program which only input IP/username/password, so the argument is always the same.

@bmiklautz
Copy link
Member

@macarthor thank you for clarifying - thought that you use git ;). Commit information doesn't exist in the zip - only way is if you have a look to the web interface and see what the last commit was.

I've done the tests, as described in your first post:
xfreerdp /bpp:16 /f /fonts /compression /cert-ignore /sound:sys:alsa /drive:SharedFolder,/tmp /:user /p:pass /v:win7testbox

@bmiklautz
Copy link
Member

@macarthor was able to reproduce with classic theme! - Thanks.

@macarthor
Copy link
Author

@bmiklautz thanks god... waiting your patch...

bmiklautz added a commit to bmiklautz/FreeRDP that referenced this issue Sep 24, 2013
@bmiklautz
Copy link
Member

@macarthor would you try if https://github.com/bmiklautz/FreeRDP/tree/issue/1500 fixes the issue for you?

@ghost ghost assigned bmiklautz Sep 24, 2013
@macarthor
Copy link
Author

@bmiklautz this patch works in both Windows Classic and Aero Architecture themes. I've tested opening 10+ times of tortoise svn GUI in both themes. many thanks! Before these patch, I am scared of opening svn GUI...

@haiq2006
Copy link

Just think the circumstance I have mentened in Issue #1428: xfreerdp crash:
*** variable data not existing in fast glyph message, cbData/glyph->aj is containing old invalid data ****

Because every time such message comes, update_gdi_fast_glyph() will enter the following branch:
if (fast_glyph->cbData > 1)
{
glyph->aj = glyph_data->aj
...
glyph_cache_put(cache->glyph, fast_glyph->cacheId, fast_glyph->data[0], glyph);
}

glyph_data->aj is inavlid at this time, glyph_cache_put() will put the invalid pointer at glyph_cache->glyphCache[id].entries[index], and the valid prevGlyph is wrongly free(). When the second message without variable data comes, a double free occurs.

bmiklautz's patch cloud work around this error. because it will allocate memory everytime before calling glyph_cache_put(). But actually this is just a duplicate copy, not doing glyph_cache_put() seem to be a better solution.

@macarthor
Copy link
Author

@bmiklautz plz refer to haiq2006's comment above. he did not @ you.

@bmiklautz
Copy link
Member

@haiq2006 the memory allocation shouldn't hurt since it isn't that much and not a that often called order ;) but you are absolutely right - it doesn't make sense. - I'll have another look to the spec [MS-RDPEGDI] if this behaviour is correct or not.
@macarthor thank you but since I'm assigned I get a notification on every update ;)

@macarthor
Copy link
Author

@bmiklautz @haiq2006 glyph_data->aj is wrongly freed? glyph->aj should have been pointed to glyph_data->aj in correct logic?

@bmiklautz
Copy link
Member

@haiq2006 according to the spec only fields that have change are sent over the wire. If a field is not present this means it's value is the same value as the last value sent for that order ([MS-RDPEGDI] 2.2.2.2.1.1.2)
As a result the VariableBytes from the last order must be used if no new VariableBytes are received within the order. So it is possible that the same glyph data is put into the cache twice and therefore the newly allocated memory (and the glyph_cache_put) is correct since we need to ensure that the glyph is at the cache position where the server expects it.

@haiq2006
Copy link

hi @bmiklautz you are right at [MS-RDPEGDI]. The protocol implys a memcpy of glyph cache when variable data not exist(reusing the value in last operation). This memcpy is useful when cacheId changes but variable data not exist.

@bmiklautz
Copy link
Member

@haiq2006 thanks for checking!
To catch the case where the cacheid didn't change we could add some tracking of the cacheid but I think it's not worth the effort since it's only a small amount of memory so the memcpy might be quicker than the tracking ;).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants