Bug 212412 – REGRESSION(r260318): [WPE][GTK] Uninitialized memory read in MemoryPressureMonitor

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

Hm, I don't fully understand this one:

==9020== Thread 46 PressureMonitor:
==9020== Conditional jump or move depends on uninitialised value(s)
==9020==    at 0x6AD5D77: systemMemoryUsedAsPercentage (MemoryPressureMonitor.cpp:246)
==9020==    by 0x6AD5D77: WebKit::MemoryPressureMonitor::start()::{lambda()#1}::operator()() const [clone .constprop.0] (MemoryPressureMonitor.cpp:347)
==9020==    by 0x6AD6358: WTF::Detail::CallableWrapper<WebKit::MemoryPressureMonitor::start()::{lambda()#1}, void>::call() (Function.h:52)
==9020==    by 0xA4ACE98: operator() (Function.h:84)
==9020==    by 0xA4ACE98: WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) (Threading.cpp:167)
==9020==    by 0xA4F83A8: WTF::wtfThreadEntryPoint(void*) (ThreadingPOSIX.cpp:197)
==9020==    by 0xB1AB431: start_thread (pthread_create.c:477)
==9020==    by 0x58BF9D2: clone (clone.S:95)
==9020==  Uninitialised value was created by a stack allocation
==9020==    at 0x6AD5BAB: WebKit::MemoryPressureMonitor::start()::{lambda()#1}::operator()() const [clone .constprop.0] (MemoryPressureMonitor.cpp:330)

Zero-initializing the token buffer avoids the warning. I'm not sure, but I *think* it's a false positive. token should be initialized by fscanf up through the first NUL character. Past that can be uninitialized, but that shouldn't affect program execution. So I *think* it's not a real bug, though we should certainly initialize the buffer to suppress the warning.

[Michael Catanzaro]

Created attachment 400342 [details]
Patch

[Adrian Perez]

Comment on attachment 400342 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400342&action=review

> Source/WebKit/ChangeLog:8
> +        I think this is a false-positive, but let's suppress the warning by zero-initializing this

Seems so…

> Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp:234
> +        char token[MEMINFO_TOKEN_BUFFER_SIZE + 1] = { 0 };

…because the “token” variable is only used if the “fscanf()” call below
returns “2”, meaning that it successfully scanned two elements, and when
it scans a string, at always adds a terminating '\0' to the buffer.

Anyhoo, let's land this, initializing the variable is good.

[EWS]

Committed r262217: <https://trac.webkit.org/changeset/262217>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400342 [details].