Bug 261497 - REGRESSION(r255164) [PlayStation] WTFReportBacktrace tries to print backtrace even when backtrace cannot be obtained and crashes
Summary: REGRESSION(r255164) [PlayStation] WTFReportBacktrace tries to print backtrace...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Local Build
Hardware: Other Other
: P2 Normal
Assignee: Tomoki Imai
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-09-13 00:51 PDT by Tomoki Imai
Modified: 2023-09-19 06:17 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tomoki Imai 2023-09-13 00:51:22 PDT
When !HAVE(BACKTRACE) && !OS(WINDOWS),
- WTFGetBacktrace(samples, &frames) make frames = 0
- WTFReportBacktraceWithPrefixAndPrintStream passes -2 (=frames-framesToSkip) to WTFPrintBacktraceWithPrefixAndPrintStream.
- WTFPrintBacktraceWithPrefixAndPrintStream static_cast -2 to size_t, which can overflow and make large number.
- It possibly tries to print the large stack and eventually crashes.

void WTFReportBacktraceWithPrefixAndPrintStream(PrintStream& out, const char* prefix)
{
    static constexpr int framesToShow = 31;
    static constexpr int framesToSkip = 2;
    void* samples[framesToShow + framesToSkip];
    int frames = framesToShow + framesToSkip;

    WTFGetBacktrace(samples, &frames);
    WTFPrintBacktraceWithPrefixAndPrintStream(out, samples + framesToSkip, frames - framesToSkip, prefix);
}

https://github.com/WebKit/WebKit/blob/f33e99829e4f572a15eb8c2a6ca3d78fa227e9cc/Source/WTF/wtf/Assertions.cpp#L298-L307

void WTFGetBacktrace(void** stack, int* size)
{
#if HAVE(BACKTRACE)
    *size = backtrace(stack, *size);
#elif OS(WINDOWS)
    *size = RtlCaptureStackBackTrace(0, *size, stack, nullptr);
#else
    UNUSED_PARAM(stack);
    *size = 0;
#endif
}

https://github.com/WebKit/WebKit/blob/f33e99829e4f572a15eb8c2a6ca3d78fa227e9cc/Source/WTF/wtf/StackTrace.cpp#L34-L44

void WTFPrintBacktraceWithPrefixAndPrintStream(PrintStream& out, void** stack, int size, const char* prefix)
{
    out.print(StackTracePrinter { { stack, static_cast<size_t>(size) }, prefix });
}

https://github.com/WebKit/WebKit/blob/f33e99829e4f572a15eb8c2a6ca3d78fa227e9cc/Source/WTF/wtf/Assertions.cpp#L309-L312
Comment 1 Tomoki Imai 2023-09-13 00:52:52 PDT
Note: 

WTF::StackTrace::captureStackTrace has similar code, but it blocks the code by "static_cast<size_t>(capturedFrames) > framesToSkip".

std::unique_ptr<StackTrace> StackTrace::captureStackTrace(size_t maxFrames, size_t framesToSkip)
{
    static_assert(sizeof(StackTrace) == sizeof(void*) * 3);
    // We overwrite the memory of the two first skipped frames, m_stack[0] will hold the third one.
    static_assert(offsetof(StackTrace, m_stack) == sizeof(void*) * 2);

    maxFrames = std::max<size_t>(1, maxFrames);
    // Skip 2 additional frames i.e. StackTrace::captureStackTrace and WTFGetBacktrace.
    framesToSkip += 2;
    size_t capacity = maxFrames + framesToSkip;
    void** storage = static_cast<void**>(fastMalloc(capacity * sizeof(void*)));
    size_t size = 0;
    size_t initialFrame = 0;
    int capturedFrames = static_cast<int>(capacity);
    WTFGetBacktrace(storage, &capturedFrames);
    if (static_cast<size_t>(capturedFrames) > framesToSkip) {
        size = static_cast<size_t>(capturedFrames) - framesToSkip;
        initialFrame = framesToSkip - 2; 
    }
    return std::unique_ptr<StackTrace> { new (NotNull, storage) StackTrace(size, initialFrame) };
}

https://github.com/WebKit/WebKit/blob/f33e99829e4f572a15eb8c2a6ca3d78fa227e9cc/Source/WTF/wtf/StackTrace.cpp#L48-L68
Comment 2 Tomoki Imai 2023-09-13 01:39:16 PDT
Pull request: https://github.com/WebKit/WebKit/pull/17726
Comment 3 EWS 2023-09-19 06:16:41 PDT
Committed 268121@main (a4279526dfa5): <https://commits.webkit.org/268121@main>

Reviewed commits have been landed. Closing PR #17726 and removing active labels.
Comment 4 Radar WebKit Bug Importer 2023-09-19 06:17:14 PDT
<rdar://problem/115721690>