Bug 7713 - valgrind warning reading uninitialized part of xEvent when calling XWithdrawWindow (XSendEvent)
Summary: valgrind warning reading uninitialized part of xEvent when calling XWithdrawW...
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Lib/Xlib (show other bugs)
Version: unspecified
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2006-07-31 15:36 UTC by David Baron
Modified: 2007-03-29 07:32 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

Attachments
patch (444 bytes, patch)
2007-01-17 15:25 UTC, David Baron 		no flags Details | Splinter Review
Maybe nicer patch (497 bytes, patch)
2007-02-20 23:29 UTC, Tilman Sauerbeck 		no flags Details | Splinter Review
View All

Description David Baron 2006-07-31 15:36:40 UTC 
When I close a modal dialog in Firefox (running with --sync), I get the
following valgrind warning (trimmed addresses to fit neatly):

Syscall param write(buf) points to uninitialised byte(s)
   at (within /lib/libpthread-2.4.so)
   by _X11TransWrite (/usr/include/X11/Xtrans/Xtrans.c:897)
   by _XFlushInt (/usr/src/debug/libX11-1.0.0/src/XlibInt.c:653)
   by _XReply (/usr/src/debug/libX11-1.0.0/src/XlibInt.c:1692)
   by XSync (/usr/src/debug/libX11-1.0.0/src/Sync.c:48)
   by _XSyncFunction (/usr/src/debug/libX11-1.0.0/src/Synchro.c:37)
   by XSendEvent (/usr/src/debug/libX11-1.0.0/src/SendEvent.c:77)
   by XWithdrawWindow (/usr/src/debug/libX11-1.0.0/src/Withdraw.c:81)
   by gdk_window_withdraw (/builds/gtk2/cvs-gnome/gtk+/gdk/x11/gdkwindow-x11.c:1636)
   ...

This appears to be because XSendEvent sends an entire xEvent struct as part of
the event, but _XEventToWire (I presume) has only filled in the non-padding
parts of the xEvent.u.unmapNotify.

It would be good to zero-fill the padding in the event so that uninitialized
data is not written to a socket.  This avoids spurious valgrind warnings when
running programs that use X -- warnings that are rather hard to filter out when
not running with --sync.
Comment 1 David Baron 2007-01-17 15:25:06 UTC 
Created attachment 8436 [details] [review]
patch

I tested that this patch fixes the valgrind warnings from Mozilla's session
restore dialog in libX11 pulled from git this morning, built with --without-xcb
(since xsltproc wouldn't behave during xcb compilation).
Comment 2 John Davidorff Pell 2007-02-20 00:27:52 UTC 
This also has the added benefit of allowing compressed X connections (i.e. over ssh w/ compression enabled) to more succinctly compress the zero-filled bytes. :-) Yay!

JP
Comment 3 Tilman Sauerbeck 2007-02-20 23:29:42 UTC 
Created attachment 8796 [details] [review]
Maybe nicer patch

To make this patch look less like dangerous, we might want to move the memset() to the caller of _XEventToWire(), which is XSendEvent(). Like this. Untested.
Comment 4 Daniel Stone 2007-02-27 01:33:03 UTC 
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 5 Tilman Sauerbeck 2007-03-28 12:22:11 UTC 
Fixed in ab0bcd07957cecc8e7c0e75d5160a625e91264fe.
Comment 6 Jie Luo 2007-03-29 05:30:12 UTC 
I think the prototype of memset is

void *memset(void *s, int c, size_t n)

So maybe this patch should change to 

memset (&ev, 0, sizeof(ev));
Comment 7 Tilman Sauerbeck 2007-03-29 07:28:00 UTC 
Man, I suck.

I knew that error was in the patch that I attached, but I didn't bother to mention it here since it's so obvious. I also tested the modified version, but when I committed the patch, I grabbed the original from bugzilla.

Thanks for spotting that, Jie Luo.
Comment 8 Tilman Sauerbeck 2007-03-29 07:32:06 UTC 
Fixed the fix in 0994faa0c76c45b106442db461b8a30a3e1c9395.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.