From e60c2914c354d4725d170e4fe4f14456caccd3cc Mon Sep 17 00:00:00 2001 From: Tim Wiederhake Date: Sat, 16 Mar 2024 13:07:19 +0100 Subject: Fix memory leak in SaveYourselfPhase2CB Valgrind results before: definitely lost: 981 bytes in 37 blocks indirectly lost: 872 bytes in 6 blocks possibly lost: 0 bytes in 0 blocks still reachable: 991,316 bytes in 1,795 blocks suppressed: 0 bytes in 0 blocks Valgrind results after: definitely lost: 800 bytes in 23 blocks indirectly lost: 872 bytes in 6 blocks possibly lost: 0 bytes in 0 blocks still reachable: 991,316 bytes in 1,795 blocks suppressed: 0 bytes in 0 blocks Signed-off-by: Tim Wiederhake Part-of: --- src/session.c | 116 +++++++++++++++++++++++++++++++--------------------------- src/session.h | 1 + src/twm.c | 2 + 3 files changed, 65 insertions(+), 54 deletions(-) diff --git a/src/session.c b/src/session.c index 92b25bb..af2b437 100644 --- a/src/session.c +++ b/src/session.c @@ -67,6 +67,7 @@ static XtInputId iceInputId; static char *twm_clientId; static TWMWinConfigEntry *winConfigHead = NULL; static Bool sent_save_done = 0; +static SmProp *props[5]; #define SAVEFILE_VERSION 2 @@ -676,8 +677,6 @@ SaveYourselfPhase2CB(SmcConn smcConn2, SmPointer clientData _X_UNUSED) const char *path; char *filename = NULL; Bool success = False; - SmProp prop1, prop2, prop3, *props[3]; - SmPropValue prop1val, prop2val, prop3val; char discardCommand[80]; int numVals, i; static int first_time = 1; @@ -688,36 +687,38 @@ SaveYourselfPhase2CB(SmcConn smcConn2, SmPointer clientData _X_UNUSED) if (first_time) { char userId[20]; - char hint = SmRestartIfRunning; + char *hint; - prop1.name = strdup(SmProgram); - prop1.type = strdup(SmARRAY8); - prop1.num_vals = 1; - prop1.vals = &prop1val; - prop1val.value = Argv[0]; - prop1val.length = (int) strlen(Argv[0]); + props[0] = (SmProp *) calloc(1, sizeof(SmProp)); + props[0]->name = strdup(SmProgram); + props[0]->type = strdup(SmARRAY8); + props[0]->num_vals = 1; + props[0]->vals = (SmPropValue *) calloc(1, sizeof(SmPropValue)); + props[0]->vals[0].value = strdup(Argv[0]); + props[0]->vals[0].length = (int) strlen(Argv[0]); snprintf(userId, sizeof(userId), "%ld", (long) getuid()); - prop2.name = strdup(SmUserID); - prop2.type = strdup(SmARRAY8); - prop2.num_vals = 1; - prop2.vals = &prop2val; - prop2val.value = (SmPointer) userId; - prop2val.length = (int) strlen(userId); - - prop3.name = strdup(SmRestartStyleHint); - prop3.type = strdup(SmCARD8); - prop3.num_vals = 1; - prop3.vals = &prop3val; - prop3val.value = (SmPointer) & hint; - prop3val.length = 1; - - props[0] = &prop1; - props[1] = &prop2; - props[2] = &prop3; - SmcSetProperties(smcConn2, 3, props); + props[1] = (SmProp *) calloc(1, sizeof(SmProp)); + props[1]->name = strdup(SmUserID); + props[1]->type = strdup(SmARRAY8); + props[1]->num_vals = 1; + props[1]->vals = (SmPropValue *) calloc(1, sizeof(SmPropValue)); + props[1]->vals[0].value = strdup(userId); + props[1]->vals[0].length = (int) strlen(userId); + + hint = (char *) malloc(1); + hint[0] = SmRestartIfRunning; + + props[2] = (SmProp *) calloc(1, sizeof(SmProp)); + props[2]->name = strdup(SmRestartStyleHint); + props[2]->type = strdup(SmCARD8); + props[2]->num_vals = 1; + props[2]->vals = (SmPropValue *) calloc(1, sizeof(SmPropValue)); + props[2]->vals[0].value = hint; + props[2]->vals[0].length = 1; + SmcSetProperties(smcConn2, 3, props); first_time = 0; } @@ -770,13 +771,12 @@ SaveYourselfPhase2CB(SmcConn smcConn2, SmPointer clientData _X_UNUSED) } } - prop1.name = strdup(SmRestartCommand); - prop1.type = strdup(SmLISTofARRAY8); - - prop1.vals = (SmPropValue *) - malloc((size_t) (Argc + 4) * sizeof(SmPropValue)); + props[3] = (SmProp *) calloc(1, sizeof(SmProp)); + props[3]->name = strdup(SmRestartCommand); + props[3]->type = strdup(SmLISTofARRAY8); + props[3]->vals = (SmPropValue *) calloc(Argc + 4, sizeof(SmPropValue)); - if (!prop1.vals) { + if (!props[3]->vals) { success = False; goto bad; } @@ -789,38 +789,36 @@ SaveYourselfPhase2CB(SmcConn smcConn2, SmPointer clientData _X_UNUSED) i++; } else { - prop1.vals[numVals].value = (SmPointer) Argv[i]; - prop1.vals[numVals++].length = (int) strlen(Argv[i]); + props[3]->vals[numVals].value = (SmPointer) strdup(Argv[i]); + props[3]->vals[numVals++].length = (int) strlen(Argv[i]); } } - prop1.vals[numVals].value = strdup("-clientId"); - prop1.vals[numVals++].length = 9; + props[3]->vals[numVals].value = strdup("-clientId"); + props[3]->vals[numVals++].length = 9; - prop1.vals[numVals].value = strdup(twm_clientId); - prop1.vals[numVals++].length = (int) strlen(twm_clientId); + props[3]->vals[numVals].value = strdup(twm_clientId); + props[3]->vals[numVals++].length = (int) strlen(twm_clientId); - prop1.vals[numVals].value = strdup("-restore"); - prop1.vals[numVals++].length = 8; + props[3]->vals[numVals].value = strdup("-restore"); + props[3]->vals[numVals++].length = 8; - prop1.vals[numVals].value = strdup(filename); - prop1.vals[numVals++].length = (int) strlen(filename); + props[3]->vals[numVals].value = strdup(filename); + props[3]->vals[numVals++].length = (int) strlen(filename); - prop1.num_vals = numVals; + props[3]->num_vals = numVals; snprintf(discardCommand, sizeof(discardCommand), "rm %s", filename); - prop2.name = strdup(SmDiscardCommand); - prop2.type = strdup(SmARRAY8); - prop2.num_vals = 1; - prop2.vals = &prop2val; - prop2val.value = (SmPointer) discardCommand; - prop2val.length = (int) strlen(discardCommand); - props[0] = &prop1; - props[1] = &prop2; + props[4] = (SmProp *) calloc(1, sizeof(SmProp)); + props[4]->name = strdup(SmDiscardCommand); + props[4]->type = strdup(SmARRAY8); + props[4]->num_vals = 1; + props[4]->vals = (SmPropValue *) calloc(1, sizeof(SmPropValue)); + props[4]->vals[0].value = strdup(discardCommand); + props[4]->vals[0].length = (int) strlen(discardCommand); - SmcSetProperties(smcConn2, 2, props); - free(prop1.vals); + SmcSetProperties(smcConn2, 2, props + 3); bad: SmcSaveYourselfDone(smcConn2, success); @@ -921,3 +919,13 @@ ConnectToSessionManager(char *previous_id, XtAppContext appContext) (XtPointer) XtInputReadMask, ProcessIceMsgProc, (XtPointer) iceConn); } + +void +DestroySession(void) +{ + for (int i = 0; i < 5; ++i) { + if (props[i]) { + SmFreeProperty(props[i]); + } + } +} diff --git a/src/session.h b/src/session.h index df91469..489ec0e 100644 --- a/src/session.h +++ b/src/session.h @@ -38,6 +38,7 @@ extern int GetWindowConfig(TwmWindow *theWindow, short *x, short *y, Bool *width_ever_changed_by_user, Bool *height_ever_changed_by_user); extern void ReadWinConfigFile(char *filename); +extern void DestroySession(void); extern SmcConn smcConn; diff --git a/src/twm.c b/src/twm.c index 4be177c..1221bf8 100644 --- a/src/twm.c +++ b/src/twm.c @@ -905,6 +905,8 @@ Done(XtPointer client_data _X_UNUSED, XtSignalId *si2 _X_UNUSED) Reborder(CurrentTime); XCloseDisplay(dpy); } + + DestroySession(); exit(EXIT_SUCCESS); } -- cgit v1.2.3 From a62500505d57a4dc2c77bbbd7d0bfb832a5aa857 Mon Sep 17 00:00:00 2001 From: Tim Wiederhake Date: Sat, 16 Mar 2024 13:07:19 +0100 Subject: Fix memory leak in FindFontSet Valgrind results before: definitely lost: 800 bytes in 23 blocks indirectly lost: 872 bytes in 6 blocks possibly lost: 0 bytes in 0 blocks still reachable: 991,316 bytes in 1,795 blocks suppressed: 0 bytes in 0 blocks Valgrind results after: definitely lost: 200 bytes in 17 blocks indirectly lost: 0 bytes in 0 blocks possibly lost: 0 bytes in 0 blocks still reachable: 918,812 bytes in 994 blocks suppressed: 0 bytes in 0 blocks Signed-off-by: Tim Wiederhake Part-of: --- src/twm.c | 20 ++++++++++++++++++++ src/util.c | 19 +++++++++++++++++++ src/util.h | 1 + 3 files changed, 40 insertions(+) diff --git a/src/twm.c b/src/twm.c index 1221bf8..d383873 100644 --- a/src/twm.c +++ b/src/twm.c @@ -813,6 +813,25 @@ CreateFonts(void) Scr->HaveFonts = TRUE; } +static void +DestroyFonts(void) +{ + for (int i = 0; i < NumScreens; ++i) { + ScreenInfo *scr = ScreenList[i]; + + if (!scr) { + continue; + } + + DestroyFont(&scr->TitleBarFont); + DestroyFont(&scr->MenuFont); + DestroyFont(&scr->IconFont); + DestroyFont(&scr->SizeFont); + DestroyFont(&scr->IconManagerFont); + DestroyFont(&scr->DefaultFont); + } +} + void RestoreWithdrawnLocation(TwmWindow *tmp) { @@ -903,6 +922,7 @@ Done(XtPointer client_data _X_UNUSED, XtSignalId *si2 _X_UNUSED) { if (dpy) { Reborder(CurrentTime); + DestroyFonts(); XCloseDisplay(dpy); } diff --git a/src/util.c b/src/util.c index 9a04ac1..52f6148 100644 --- a/src/util.c +++ b/src/util.c @@ -607,6 +607,7 @@ FindFontSet(MyFont *font, const char *fontname) twmVerbose("font for charset %s is lacking.", missing_charset_list_return[i]); } + XFreeStringList(missing_charset_list_return); } font_extents = XExtentsOfFontSet(font->fontset); @@ -672,6 +673,24 @@ GetFont(MyFont *font) } } +void +DestroyFont(MyFont *font) +{ + if (!font) { + return; + } + + if (font->fontset) { + XFreeFontSet(dpy, font->fontset); + font->fontset = NULL; + } + + if (font->font) { + XFreeFont(dpy, font->font); + font->font = NULL; + } +} + int MyFont_TextWidth(MyFont *font, const char *string, int len) { diff --git a/src/util.h b/src/util.h index 6adeed9..a7ded89 100644 --- a/src/util.h +++ b/src/util.h @@ -77,6 +77,7 @@ extern void LocateStandardColormaps(void); extern void GetColor(int kind, Pixel *what, const char *name); extern void GetColorValue(int kind, XColor *what, const char *name); extern void GetFont(MyFont *font); +extern void DestroyFont(MyFont *font); extern int MyFont_TextWidth(MyFont *font, const char *string, int len); extern void MyFont_DrawImageString(Display *dpy, Drawable d, MyFont *font, GC gc, int x, int y, const char *string, -- cgit v1.2.3 From 64edeaaa4c7492d090b4069de2eae5f411ff1cd4 Mon Sep 17 00:00:00 2001 From: Tim Wiederhake Date: Sat, 16 Mar 2024 13:07:19 +0100 Subject: Fix read from uninitialized data Syscall param writev(vector[0]) points to uninitialised byte(s) at 0x4B836C0: writev (writev.c:26) by 0x4C74FBF: ??? (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0) by 0x4C753B0: xcb_writev (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0) by 0x48B2A24: _XSend (in /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0) by 0x48B3088: _XReply (in /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0) by 0x48AE6FE: XSync (in /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0) by 0x11C523: HandleEnterNotify (events.c:2112) by 0x117847: DispatchEvent (events.c:335) by 0x117921: HandleEvents (events.c:363) by 0x12FCBD: main (twm.c:648) Address 0x5036874 is 148 bytes inside a block of size 16,384 alloc'd at 0x48459F3: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x48A23AA: XOpenDisplay (in /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0) by 0x49E5A12: XtOpenDisplay (in /usr/lib/x86_64-linux-gnu/libXt.so.6.0.0) by 0x12EB45: main (twm.c:319) Uninitialised value was created by a stack allocation at 0x12828E: send_clientmessage (menus.c:2861) Signed-off-by: Tim Wiederhake Part-of: --- src/menus.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/menus.c b/src/menus.c index 41f5bad..7814771 100644 --- a/src/menus.c +++ b/src/menus.c @@ -2860,6 +2860,7 @@ static void send_clientmessage(Window w, Atom a, Time timestamp) { XClientMessageEvent ev; + memset(&ev, '\0', sizeof(ev)); ev.type = ClientMessage; ev.window = w; -- cgit v1.2.3