Skip to content

Out-of-bounds read in compile_spirv_module()'s error-printing path - on non-null-terminated shader data #6881

Description

@marcin-sochacki

Description

When a Vulkan compute shader fails to compile via compile_spirv_module()
(src/gpu.cpp), the diagnostic error-printing code treats the shader source as
a null-terminated C string, but it isn't one — this causes an out-of-bounds
read that produces garbled, misleading diagnostic output, and reads past the
end of the intended buffer into unrelated adjacent memory.

Root cause

In src/gpu.cpp, compile_spirv_module()'s failure path (currently ~line
6337-6353 on master):

const char* p = comp_datas[3];
const char* line_end;
int line_number = 1;

while ((line_end = strchr(p, '\n')) != NULL)
{
    NCNN_LOGE("%d:\t%.*s", line_number++, (int)(line_end - p), p);
    p = line_end + 1;
}

if (*p != '\0')
{
    NCNN_LOGE("%d:\t%s", line_number, p);
}

comp_datas[3] points into a static const char whatever_comp_data[] array,
generated at build time by cmake/ncnn_generate_shader_comp_header.cmake. That
script hex-encodes the raw shader source bytes with no terminating NUL byte
added:

file(WRITE ${SHADER_COMP_HEADER} "static const char 
${SHADER_SRC_NAME_WE}_comp_data[] = {${comp_data_hex}};\n")

Since strchr() and the *p != '\0' check both require a null-terminated string,
and none is guaranteed here, both calls can read past the end of the intended
array. In practice these per-shader arrays are laid out contiguously in
.rodata by the linker, so strchr often walks harmlessly through the start of
the next embedded shader's data (which conveniently starts with #version,
ASCII #) — but it isn't bounded, and can end up printing arbitrary adjacent
memory.

Impact

It corrupts the diagnostic output every time any shader fails to compile, on any platform - making it
harder to debug the real underlying problem.
In principle it's also a genuine out-of-bounds read, not just a cosmetic issue.

Suggested fix

The exact length of comp_datas[3] is already known (comp_data_sizes[3],
computed earlier in the same function) — use it instead of relying on
null-termination, e.g. bound the scan with memchr()/an explicit end pointer
rather than strchr(), or simply add a terminating NUL byte when generating the
.comp_data[] arrays.

Environment

Reproduced with ncnn release 20260526, NCNN_SYSTEM_GLSLANG=ON, but the buggy
code is unchanged on current master and the bug isn't specific to that build
flag — it can be triggered by any shader compile failure on any platform,
since it's purely in the error-printing path.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions