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.

Sign up for GitHub

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

Jump to bottom

Pr/fix lib x11 compilation warnings #2

Closed
wants to merge 212 commits into from
Closed

Pr/fix lib x11 compilation warnings #2

wants to merge 212 commits into from

Conversation

uli42
Copy link
Owner

@uli42 uli42 commented Oct 15, 2016 •
edited
Loading

Fix some (but not all) compilation regarding libX11. I have also opened issues ArcticaProject#226 and ArcticaProject#228 for some other warnings.

apply after ArcticaProject#222

@uli42 uli42 force-pushed the pr/fix_libX11_compile_warnings branch from 3592452 to b7e1d6f Compare October 15, 2016 20:54
fooishbar and others added 29 commits October 16, 2016 01:32
@fooishbar @uli42
XStringToKeysym: Special case for XF86 keysyms
902cba2 
Some XFree86 keysyms were in XKeysymDB as XF86_foo, despite really being
XF86foo.  So, if we get to the bottom of XStringToKeysym and haven't
found our XF86_foo, try it again as XF86foo.

Signed-off-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@fooishbar @uli42
Delete now-redundant XKeysymDB
0f6b0da 
    Delete now-redundant XKeysymDB

    Since XStringToKeysym now supports all the vendor keysyms, just delete
    our XKeysymDB, which was incomplete at best, misleading at worst, and
    always an annoyance.

    Signed-off-by: Daniel Stone <daniel@fooishbar.org>
    Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
    Backported-to-NX-by: Ulrich Sibiller <uli42@gmx.de>
@fooishbar @uli42
XStringToKeysym: Check strdup() return value
1059364 
Signed-off-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Keith Packard <keithp@keithp.com>
@fooishbar @uli42
XStringToKeysym: Cope with 0x1234cafe-style input
98e6f96 
If we get input in the style of 0xdeadbeef, just return that exact
keysym.  Introduces a dependency on strtoul, which I'm told is OK on all
the systems we care about.

Signed-off-by: Daniel Stone <daniel@fooishbar.org>
@alanc @uli42
LINEAR_RGB_InitSCCData: When malloc fails, don't try to free unalloca…
defc8de 
…ted bits

One of the malloc failure checks had a goto to the wrong spot in the
list of cleanup free() calls to unwind at the end, and was freeing
bits that hadn't been initialized/allocated yet, since they would be
stored in the struct that just failed to be allocated.

Error: Null pointer dereference (CWE 476)
   Read from pointer that could be constant 'NULL'
        at line 805 of /export/alanc/X.Org/sx86/lib/libX11/nx-X11/lib/X11/LRGB.c in function 'LINEAR_RGB_InitSCCData'.
          Pointer checked against constant 'NULL' at line 754 but does not protect the dereference.

[ This bug was found by the Parfait bug checking tool.
  For more information see http://research.sun.com/projects/parfait ]

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@uli42
Initialize event type
ae72fab 
If we receive unsupported event closing connection triggers valgrind
error.

==12017== Conditional jump or move depends on uninitialised value(s)
==12017==    at 0x487D454: _XFreeDisplayStructure (OpenDis.c:607)
==12017==    by 0x486857B: XCloseDisplay (ClDisplay.c:72)
*snip*
==12017==  Uninitialised value was created by a heap allocation
==12017==    at 0x4834C48: malloc (vg_replace_malloc.c:236)
==12017==    by 0x4894147: _XEnq (XlibInt.c:877)
==12017==    by 0x4891BF3: handle_response (xcb_io.c:335)
==12017==    by 0x4892263: _XReply (xcb_io.c:626)
*snip*

Problem is that XFreeDisplaySturture is checking for qelt->event.type ==
GenericEvent while _XUnknownWireEvent doesn't store the type.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Pauli Nieminen <ext-pauli.nieminen@nokia.com>
@eras @uli42
Using freed pointer "e"
92d7ad7 
Reordered code to first to do the comparison and then to release data

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
@eras @uli42
Dereferencing possibly NULL "str" in call to function "memcpy" (Deref…
f8bb7ea 
… assumed on the basis of 'nonnull' parameter attribute.)

If _XkbGetReadBufferPtr returns NULL, goto BAILOUT

Reviewed-by: Dirk Wallenstein <halsmit@t-online.de>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
@eras @uli42
Variable "entry" tracked as NULL was dereferenced.
830b496 
Check entry for non-nullness before dereferencing it

Reviewed-by: Dirk Wallenstein <halsmit@t-online.de>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
@eras @uli42
Comparing array against NULL is not useful "&xkb->server->vmods != NULL"
610627d 
Removed superfluous comparison.

Reviewed-by: Dirk Wallenstein <halsmit@t-online.de>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
@alanc @uli42
XDefaultOMIF.c: Fix memory leaks in get_font_name
e169ea2 
Instead of copying the value returned by get_prop_name and then releasing it,
directly use the return value of get_prop_name, which allocates memory for the
name.

If get_prop_name returns NULL, continue on to XFreeFont to release the font
before returning the NULL via the normal function return.

Reviewed-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@eras @uli42
xcmx/cmxColNm: Removed unused assignments to pBuf (in two functions)
b3110de 
Pointer "pBuf" returned from "fgets(buf, 256, stream)" is never used

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@eras @uli42
xlibi18n/lcGeneric: Initialize uninitialized local variable
e4ca03f 
Using uninitialized value "new"

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@eras @uli42
ImUtil: Handle a memory leak in one early return branch
ad12055 
Fixed memory leak by adding Xfree for image

Variable "image" goes out of scope

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@eras @uli42
Xrm: NEWTABLE had a memory leak after a memory allocation error
2f73378 
The NEWTABLE macro missed freeing its allocated memory on subsequent
memory allocation errors. Added call to Xfree.

Variable "table" goes out of scope

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@uli42
xcms/LRGB: Fix potential resource leak.
cc90917 
property_return was not free'd if the allocation of pRedTbl failed.

Reviewed-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@uli42
xcms/LRGB: Add a label for freeing property_return.
3ae70a4 
The rest of the code uses goto's to free memory allocated later
and prevent memory leaks, but there were several paths were
property_return was free'd just before a goto.

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@uli42
XlibInt: info_list->watch_data was reallocated, but result was discarded
792bb8a 
info_list->watch_data was being reallocated, but the return value of
the reallocation was stored only into a local variable. This might
cause some funky behavior and crashes.

Variable "wd_array" goes out of scope
Value "wd_array" is overwritten in "wd_array = (XPointer*)realloc((char*)info_list->watch_data, (((dpy->watcher_count + 1) * 4U == 0U) ? 1U : ((dpy->watcher_count + 1) * 4U)))"

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@eras @uli42
xlibi18n/lcFile: Removed superfluous check for NULL target_dir
8b61c92 
The situation is already handled before this code.

Cannot reach dead expression "0U" inside statement "if (1U + (target_dir ? strl..."

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@eras @uli42
XlibInt: Use strncpy+zero termination instead of strcpy to enforce bu…
041e660 
…ffer size

Possible overrun of 8192 byte fixed size buffer "buffer" by copying
"ext->name" without length checking

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@alanc @uli42
Expand GetSizeOfFile() macro at the one place it's called
8e034d2 
Removes XrmI.h header that only contained this single macro

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
@alanc @uli42
Xrm.c: ReadInFile: refactor fstat error handling
ed336b1 
We can simplify the fstat failure case now that the GetFileSize macro
has been expanded inline.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
@uli42
xcms/LRGB: don't double-free property_return
21efc06 
property_return was free'd before and in the case the conditional is true,
the call to XcmsGetProperty failed which means that property_return wasn't
set so there is no need to free it again.

Double free of pointer "property_return" in call to "free"

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
@eras @uli42
cmsProp: don't deal with uninitialized values, fail instead
06360f6 
Properly handle the return value of XGetWindowProperty by considering
if after the loop as well.

Using freed pointer "prop_ret"

There were numerous things wrong in how this function interacted with
XGetWindowProperty.

None of the local variables were initialized and remained that way if
the call to XGetWindowProperty returned 1 (not Succeed). That doesn't
result in after_ret being initialized in which case if it happens to
be 0, the loop was exited. In that case format_ret and nitems_ret were
uninitialized and the function might return with success (but with
uninitialized pointer in prop_ret) or XcmsFailure.

As the buffer enlarging code was called only when XGetWindowProperty
failed (returned not Success), after_ret would not have been
initialized. It would have been initialized only if the
XGetWindowProperty has returned Success earlier, but in that case the
code fragment would not have been reached.

This patch alters the function to return XcmsFailure if the call to
XGetWindowProperty fails.

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Reviewed-by: Rami Ylimäki <rami.ylimaki@vincit.fi>
Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
@alanc @uli42
Clean up memory when first XCreateRegion succeeds and second fails
7bc3938 
Error: Memory leak (CWE 401)
   Memory leak of pointer 's' allocated with XCreateRegion()
        at line 387 of /export/alanc/X.Org/sx86-gcc/lib/libX11/nx-X11/lib/X11/Region.c in function 'XShrinkRegion'.
          's' allocated at line 387 with XCreateRegion().
          s leaks when s != 0 at line 387.
Error: Memory leak (CWE 401)
   Memory leak of pointer 'tra' allocated with XCreateRegion()
        at line 1452 of /export/alanc/X.Org/sx86-gcc/lib/libX11/nx-X11/lib/X11/Region.c in function 'XXorRegion'.
          'tra' allocated at line 1451 with XCreateRegion().
          tra leaks when tra != 0 at line 1451.

[ This bug was found by the Parfait 0.3.6 bug checking tool.
  For more information see http://labs.oracle.com/projects/parfait/ ]

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@eras @uli42
keyBind: Use Xcalloc to initialize allocated _XKeytrans
ca5be09 
Using uninitialized value "p->modifiers"

Small fix by using Xcalloc instead of Xmalloc

Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Reviewed-by: Alan Coopersmith <alan.coopersmith at oracle.com>
@eras @uli42
xkb: XkbPropertyPtr determined allocation success from wrong variables
7c09bef 
Cannot reach dead statement "return NULL;"

Check for the NULLness of prop->name and prop->value instead of
name and value, which was checked earlier anyway. Decided against
using strdup due to curious memory allocation functions and the
rest of the xkb not using it either.

Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Reviewed-by: Alan Coopersmith <alan.coopersmith at oracle.com>
@harshula @uli42
Add #define XK_SINHALA
0517ca7 
Add #define XK_SINHALA so that the Sinhala keysyms can be used by
the lk xkb keymap.

Signed-off-by: Harshula Jayasuriya <harshula@gmail.com>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
@uli42
Fix memory leak with broken bitmap files in XReadBitmapFileData
217f791 
Bitmap file data is read looping through the lines in the input file. If
there is extra data after the bitmap, these lines will be processed and
if this data represents another bitmap it will replace the one read
before causing the memory allocated for bits to leak.

This changes the code to stop processing the file once a bitmap was
read.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
alanc and others added 16 commits October 16, 2016 01:32
@alanc @uli42
xcms: use size_t for pointer offsets passed to strncmp
502f9cb 
instead of converting to int and back

Fixes clang warnings of the form:
HVC.c:190:43: warning: implicit conversion changes signedness: 'int' to
      'unsigned long' [-Wsign-conversion]
          if (strncmp(spec, _XcmsTekHVC_prefix, n) != 0) {
              ~~~~~~~

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
@stoeckmann @uli42
The validation of server responses avoids out of boundary accesses.
2058c18 
v2: FontNames.c  return a NULL list whenever a single
length field from the server is incohent.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
@stoeckmann @uli42
Validation of server responses in XGetImage()
ea3d9ff 
Check if enough bytes were received for specified image type and
geometry. Otherwise GetPixel and other functions could trigger an
out of boundary read later on.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
@uli42
Fix libX11 implicit declaration warnings
8462f39 
all _X11Trans* functions lead to a warnign like this:

implicit declaration of function ‘_X11TransOpenCOTSClient’ [-Wimplicit-function-declaration]

Fix it by partly reverting d3ae0b2 (which removed too much)
@uli42
Pending.c: Fix compilation warning
9abde45 
Pending.c: In function ‘XEventsQueued’:
Pending.c:44:5: warning: format ‘%p’ expects argument of type ‘void *’, but argument 3 has type ‘struct Display *’ [-Wformat=]
     fprintf(stderr, "\nXEventsQueued: Called with a display at [%p].\n", dpy);
     ^
@uli42
imLcIm.c: Fix unused variables
ed3337a 
imLcIm.c: In function ‘_XimCreateDefaultTree’:
imLcIm.c:525:11: warning: unused variable ‘cachedir’ [-Wunused-variable]
     char *cachedir = NULL;
           ^
imLcIm.c:521:35: warning: variable ‘intname’ set but not used [-Wunused-but-set-variable]
     char *name, *tmpname = NULL, *intname;
@uli42
Args.c: fix string format warning
72aaede 
warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 5 has type ‘long int’ [-Wformat=]
@uli42
Colormap.c: add cast to avoid warning
fb33fd2 
Colormap.c: In function ‘nxagentSetInstalledColormapWindows’:
Colormap.c:314:27: warning: format ‘%p’ expects argument of type ‘void *’, but argument 3 has type ‘WindowPtr’ [-Wformat=]
                           pWin, pWin -> drawable.class);
                           ^
@uli42
Display.c: remove unused variable
a4f146c 
Display.c: In function ‘nxagentCheckForPixmapFormatsCompatibility’:
Display.c:2471:8: warning: variable ‘one_match’ set but not used [-Wunused-but-set-variable]
   bool one_match = false;
        ^
@uli42
Window.c: Fix function assignment warning
4f72e67 
Window.c: In function ‘nxagentFrameBufferPaintWindow’:
Window.c:1968:31: warning: ISO C forbids assignment between function pointer and ‘void *’ [-Wpedantic]
   PaintWindowBackgroundBackup = pWin->drawable.pScreen -> PaintWindowBackground;
@uli42
nxagent: replace deprecated function call XKeycodeToKeysym
943fe90 
Keystroke.c: In function ‘nxagentCheckSpecialKeystroke’:
Keystroke.c:449:3: warning: ‘XKeycodeToKeysym’ is deprecated (declared at ../../../../exports/include/nx-X11/Xlib.h:1688) [-Wdeprecated-declarations]
   sym = XKeycodeToKeysym(nxagentDisplay, X -> keycode, index);

Fixes ArcticaProject#229
@uli42
Loop.cpp: Fix output of SetCore()
3032aba
@uli42
security.c: fix compiler warnings
986c54f
@uli42
xcmisc.c: remove HAVE_STDINT
6b9d3db 
This eliminates a warning since we do not have this definde in our
build environment.
@uli42
xkmread.c: use _X_UNUSED to avoid compiler warning when result is not…
2878eea 
… needed
@uli42
xkb.c: fix 'assignment makes integer from pointer without a cast'
3ae6aab 
xkb.c: In function ‘_GetCountedString’:
xkb.c:4405:8: warning: assignment makes integer from pointer without a cast [enabled by default]
     len= (CARD16 *)wire;
@uli42 uli42 force-pushed the pr/fix_libX11_compile_warnings branch from f6c9e0b to 3ae6aab Compare October 15, 2016 23:33
@uli42 uli42 closed this Oct 15, 2016
@uli42
Copy link
Owner Author

uli42 commented Oct 15, 2016

wrong, simply wrong

@uli42 uli42 deleted the pr/fix_libX11_compile_warnings branch November 2, 2016 20:58
uli42 added a commit that referenced this pull request Jun 19, 2019
@uli42
glyph.c: fix a read beyond end of heap buffer
aedd35c 
If compiled with -fsanitize=address this showed up when running startlxde:

==11551==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60d000018fbc at pc 0x7f270a9ed57b bp 0x7fff30ef3050 sp 0x7fff30ef2800
READ of size 204 at 0x60d000018fbc thread T0
    #0 0x7f270a9ed57a  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
    #1 0x559dafcd5c93 in FindGlyphRef ../../render/glyph.c:179
    #2 0x559dafcd705d in AddGlyph /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXglyph.c:71
    #3 0x559dafccc0ff in ProcRenderAddGlyphs ../../mi/../render/render.c:1186
    ArcticaProject#4 0x559dafcbd5a5 in ProcRenderDispatch /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXrender.c:1689
    ArcticaProject#5 0x559dafcbc4ea in Dispatch /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXdispatch.c:476
    ArcticaProject#6 0x559dafc4e9b0 in main /work/nx-libs/nx-X11/programs/Xserver/dix/main.c:353
    ArcticaProject#7 0x7f2708e1d09a in __libc_start_main ../csu/libc-start.c:308
    ArcticaProject#8 0x559dafc4f5d9 in _start (/work/nx-libs/nx-X11/programs/Xserver/nxagent+0x6e5d9)

0x60d000018fbc is located 0 bytes to the right of 140-byte region [0x60d000018f30,0x60d000018fbc)
allocated by thread T0 here:
    #0 0x7f270aa1e330 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe9330)
    #1 0x559dafcd646c in AllocateGlyph ../../render/glyph.c:348

This happens when two glyphs are compared via memcmp and the smaller
one happens to be identical to the beginning of the bigger one.

Newer render implementations use a sha1 hash instead of memcmp so this
patch will (hopefully) be obsolete once render gets updated.
uli42 added a commit that referenced this pull request Jun 19, 2019
@uli42
Keyboard.c: fix three memory leaks
81057b2 
==12976==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 6 byte(s) in 1 object(s) allocated from:
    #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
    #1 0x559ca29c5035 in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:866
    #2 0x7a29bff07  (<unknown module>)

Direct leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
    #1 0x559ca29c509a in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:870
    #2 0x7a29bff07  (<unknown module>)

Direct leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
    #1 0x559ca29c507f in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:869
    #2 0x7a29bff07  (<unknown module>)

SUMMARY: AddressSanitizer: 8 byte(s) leaked in 3 allocation(s).
uli42 added a commit that referenced this pull request Jun 21, 2019
@uli42
glyph.c: fix a read beyond end of heap buffer
53d4b74 
If compiled with -fsanitize=address this showed up when running startlxde:

==11551==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60d000018fbc at pc 0x7f270a9ed57b bp 0x7fff30ef3050 sp 0x7fff30ef2800
READ of size 204 at 0x60d000018fbc thread T0
    #0 0x7f270a9ed57a  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
    #1 0x559dafcd5c93 in FindGlyphRef ../../render/glyph.c:179
    #2 0x559dafcd705d in AddGlyph /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXglyph.c:71
    #3 0x559dafccc0ff in ProcRenderAddGlyphs ../../mi/../render/render.c:1186
    ArcticaProject#4 0x559dafcbd5a5 in ProcRenderDispatch /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXrender.c:1689
    ArcticaProject#5 0x559dafcbc4ea in Dispatch /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXdispatch.c:476
    ArcticaProject#6 0x559dafc4e9b0 in main /work/nx-libs/nx-X11/programs/Xserver/dix/main.c:353
    ArcticaProject#7 0x7f2708e1d09a in __libc_start_main ../csu/libc-start.c:308
    ArcticaProject#8 0x559dafc4f5d9 in _start (/work/nx-libs/nx-X11/programs/Xserver/nxagent+0x6e5d9)

0x60d000018fbc is located 0 bytes to the right of 140-byte region [0x60d000018f30,0x60d000018fbc)
allocated by thread T0 here:
    #0 0x7f270aa1e330 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe9330)
    #1 0x559dafcd646c in AllocateGlyph ../../render/glyph.c:348

This happens when two glyphs are compared via memcmp and the smaller
one happens to be identical to the beginning of the bigger one.

Newer render implementations use a sha1 hash instead of memcmp so this
patch will (hopefully) be obsolete once render gets updated.
uli42 added a commit that referenced this pull request Jun 21, 2019
@uli42
Keyboard.c: fix three memory leaks
51fbd72 
==12976==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 6 byte(s) in 1 object(s) allocated from:
    #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
    #1 0x559ca29c5035 in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:866
    #2 0x7a29bff07  (<unknown module>)

Direct leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
    #1 0x559ca29c509a in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:870
    #2 0x7a29bff07  (<unknown module>)

Direct leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
    #1 0x559ca29c507f in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:869
    #2 0x7a29bff07  (<unknown module>)

SUMMARY: AddressSanitizer: 8 byte(s) leaked in 3 allocation(s).
sunweaver pushed a commit that referenced this pull request Jun 22, 2019
@uli42 @sunweaver
glyph.c: fix a read beyond end of heap buffer
234be02 
If compiled with -fsanitize=address this showed up when running startlxde:

==11551==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60d000018fbc at pc 0x7f270a9ed57b bp 0x7fff30ef3050 sp 0x7fff30ef2800
READ of size 204 at 0x60d000018fbc thread T0
    #0 0x7f270a9ed57a  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
    #1 0x559dafcd5c93 in FindGlyphRef ../../render/glyph.c:179
    #2 0x559dafcd705d in AddGlyph /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXglyph.c:71
    #3 0x559dafccc0ff in ProcRenderAddGlyphs ../../mi/../render/render.c:1186
    ArcticaProject#4 0x559dafcbd5a5 in ProcRenderDispatch /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXrender.c:1689
    ArcticaProject#5 0x559dafcbc4ea in Dispatch /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXdispatch.c:476
    ArcticaProject#6 0x559dafc4e9b0 in main /work/nx-libs/nx-X11/programs/Xserver/dix/main.c:353
    ArcticaProject#7 0x7f2708e1d09a in __libc_start_main ../csu/libc-start.c:308
    ArcticaProject#8 0x559dafc4f5d9 in _start (/work/nx-libs/nx-X11/programs/Xserver/nxagent+0x6e5d9)

0x60d000018fbc is located 0 bytes to the right of 140-byte region [0x60d000018f30,0x60d000018fbc)
allocated by thread T0 here:
    #0 0x7f270aa1e330 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe9330)
    #1 0x559dafcd646c in AllocateGlyph ../../render/glyph.c:348

This happens when two glyphs are compared via memcmp and the smaller
one happens to be identical to the beginning of the bigger one.

Newer render implementations use a sha1 hash instead of memcmp so this
patch will (hopefully) be obsolete once render gets updated.
sunweaver pushed a commit that referenced this pull request Jun 22, 2019
@uli42 @sunweaver
Keyboard.c: fix three memory leaks
6da1066 
==12976==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 6 byte(s) in 1 object(s) allocated from:
    #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
    #1 0x559ca29c5035 in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:866
    #2 0x7a29bff07  (<unknown module>)

Direct leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
    #1 0x559ca29c509a in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:870
    #2 0x7a29bff07  (<unknown module>)

Direct leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
    #1 0x559ca29c507f in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:869
    #2 0x7a29bff07  (<unknown module>)

SUMMARY: AddressSanitizer: 8 byte(s) leaked in 3 allocation(s).
uli42 added a commit that referenced this pull request Oct 2, 2020
@uli42
nxagent: fix stack smashing
aa5ef6c 
In compext Atom has the size of XlibAtom. Therefore calling functions
of Compext.c requires to use/pass XlibAtom. Same for Window/XlibWindow.

==15438==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffffffcdc0 at pc 0x5555556a81b5 bp 0x7fffffffcd10 sp 0x7fffffffcd08
WRITE of size 8 at 0x7fffffffcdc0 thread T0
    #0 0x5555556a81b4 in NXGetCollectedProperty nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c:4124
    #1 0x5555557d0488 in nxagentCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Clipboard.c:1202
    #2 0x555555723340 in nxagentHandleCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:3923
    #3 0x55555571d4db in nxagentHandleProxyEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:3007
    ArcticaProject#4 0x55555571bb92 in nxagentHandleClientMessageEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:2595
    ArcticaProject#5 0x555555717dfc in nxagentDispatchEvents nx-X11/programs/Xserver/hw/nxagent/Events.c:1827
    ArcticaProject#6 0x555555750813 in nxagentBlockHandler nx-X11/programs/Xserver/hw/nxagent/Handlers.c:437
    ArcticaProject#7 0x5555556c1b5d in BlockHandler nx-X11/programs/Xserver/dix/dixutils.c:403
    ArcticaProject#8 0x5555556d47ff in WaitForSomething nx-X11/programs/Xserver/os/WaitFor.c:232
    ArcticaProject#9 0x555555665b22 in Dispatch nx-X11/programs/Xserver/hw/nxagent/NXdispatch.c:365
    ArcticaProject#10 0x5555555ed760 in main nx-X11/programs/Xserver/dix/main.c:350
    ArcticaProject#11 0x7ffff604909a in __libc_start_main ../csu/libc-start.c:308
    ArcticaProject#12 0x5555555edc09 in _start (nx-X11/programs/Xserver/nxagent+0x99c09)

Address 0x7fffffffcdc0 is located in stack of thread T0 at offset 32 in frame
    #0 0x5555557d0324 in nxagentCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Clipboard.c:1190

  This frame has 5 object(s):
    [32, 36) 'atomReturnType' <== Memory access at offset 32 partially overflows this variable
    [96, 100) 'resultFormat'
    [160, 168) 'ulReturnItems'
    [224, 232) 'ulReturnBytesLeft'
    [288, 296) 'pszReturnData'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c:4124 in NXGetCollectedProperty
...
sunweaver pushed a commit that referenced this pull request Oct 17, 2020
@uli42 @sunweaver
nxagent: fix stack smashing
ae03770 
In compext Atom has the size of XlibAtom. Therefore calling functions
of Compext.c requires to use/pass XlibAtom. Same for Window/XlibWindow.

==15438==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffffffcdc0 at pc 0x5555556a81b5 bp 0x7fffffffcd10 sp 0x7fffffffcd08
WRITE of size 8 at 0x7fffffffcdc0 thread T0
    #0 0x5555556a81b4 in NXGetCollectedProperty nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c:4124
    #1 0x5555557d0488 in nxagentCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Clipboard.c:1202
    #2 0x555555723340 in nxagentHandleCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:3923
    #3 0x55555571d4db in nxagentHandleProxyEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:3007
    ArcticaProject#4 0x55555571bb92 in nxagentHandleClientMessageEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:2595
    ArcticaProject#5 0x555555717dfc in nxagentDispatchEvents nx-X11/programs/Xserver/hw/nxagent/Events.c:1827
    ArcticaProject#6 0x555555750813 in nxagentBlockHandler nx-X11/programs/Xserver/hw/nxagent/Handlers.c:437
    ArcticaProject#7 0x5555556c1b5d in BlockHandler nx-X11/programs/Xserver/dix/dixutils.c:403
    ArcticaProject#8 0x5555556d47ff in WaitForSomething nx-X11/programs/Xserver/os/WaitFor.c:232
    ArcticaProject#9 0x555555665b22 in Dispatch nx-X11/programs/Xserver/hw/nxagent/NXdispatch.c:365
    ArcticaProject#10 0x5555555ed760 in main nx-X11/programs/Xserver/dix/main.c:350
    ArcticaProject#11 0x7ffff604909a in __libc_start_main ../csu/libc-start.c:308
    ArcticaProject#12 0x5555555edc09 in _start (nx-X11/programs/Xserver/nxagent+0x99c09)

Address 0x7fffffffcdc0 is located in stack of thread T0 at offset 32 in frame
    #0 0x5555557d0324 in nxagentCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Clipboard.c:1190

  This frame has 5 object(s):
    [32, 36) 'atomReturnType' <== Memory access at offset 32 partially overflows this variable
    [96, 100) 'resultFormat'
    [160, 168) 'ulReturnItems'
    [224, 232) 'ulReturnBytesLeft'
    [288, 296) 'pszReturnData'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c:4124 in NXGetCollectedProperty
...
uli42 added a commit that referenced this pull request Oct 18, 2020
@uli42
yConnDis.c: fix memory leak
ad9528a 
 Direct leak of 3 byte(s) in 1 object(s) allocated from:
 #0 0xb79e85d4 in __interceptor_malloc (/lib/i386-linux-gnu/libasan.so.5+0xeb5d4)
 #1 0xb770b635 in copystring /home/uli/work/nx/nx-libs/nx-X11/lib/src/ConnDis.c:96
 #2 0xb770ba56 in _X11TransConnectDisplay /home/uli/work/nx/nx-libs/nx-X11/lib/src/ConnDis.c:229
 #3 0xb776b4fd in XOpenDisplay /home/uli/work/nx/nx-libs/nx-X11/lib/src/OpenDis.c:215
 ArcticaProject#4 0x63e2fd in nxagentInternalOpenDisplay /home/uli/work/nx/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Display.c:608
 ArcticaProject#5 0x63fa03 in nxagentOpenDisplay /home/uli/work/nx/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Display.c:1140
 ArcticaProject#6 0x694b5a in InitOutput /home/uli/work/nx/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Init.c:305
 ArcticaProject#7 0x5f7b11 in main /home/uli/work/nx/nx-libs/nx-X11/programs/Xserver/dix/main.c:278
 ArcticaProject#8 0xb6f04b40 in __libc_start_main ../csu/libc-start.c:308

I have not investigated the exact location where an XFree() was missing but added multiple
Xfree() calls whereever appropriate.

Fixes ArcticaProject#951
sunweaver pushed a commit that referenced this pull request Nov 3, 2020
@uli42 @sunweaver
yConnDis.c: fix memory leak
12b22e9 
 Direct leak of 3 byte(s) in 1 object(s) allocated from:
 #0 0xb79e85d4 in __interceptor_malloc (/lib/i386-linux-gnu/libasan.so.5+0xeb5d4)
 #1 0xb770b635 in copystring /home/uli/work/nx/nx-libs/nx-X11/lib/src/ConnDis.c:96
 #2 0xb770ba56 in _X11TransConnectDisplay /home/uli/work/nx/nx-libs/nx-X11/lib/src/ConnDis.c:229
 #3 0xb776b4fd in XOpenDisplay /home/uli/work/nx/nx-libs/nx-X11/lib/src/OpenDis.c:215
 ArcticaProject#4 0x63e2fd in nxagentInternalOpenDisplay /home/uli/work/nx/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Display.c:608
 ArcticaProject#5 0x63fa03 in nxagentOpenDisplay /home/uli/work/nx/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Display.c:1140
 ArcticaProject#6 0x694b5a in InitOutput /home/uli/work/nx/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Init.c:305
 ArcticaProject#7 0x5f7b11 in main /home/uli/work/nx/nx-libs/nx-X11/programs/Xserver/dix/main.c:278
 ArcticaProject#8 0xb6f04b40 in __libc_start_main ../csu/libc-start.c:308

I have not investigated the exact location where an XFree() was missing but added multiple
Xfree() calls whereever appropriate.

Fixes ArcticaProject#951
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.