Skip to content

[bug] MultiHeadAttention::load_param divides by zero when num_heads is 0, crashing Net::load_param on any param file that sets 1=0 #6926

Description

MultiHeadAttention::load_param divides by zero when num_heads is 0, crashing Net::load_param on any param file that sets 1=0

ncnn_attachment.zip

error log

Floating point exception (core dumped)

Under a debugger the faulting instruction is the integer division in

src/layer/multiheadattention.cpp:59
scale = pd.get(6, 1.f / sqrtf(embed_dim / num_heads));

context

  • ncnn at c189d88, still present on current master
  • Built exactly as your own x86-64 CI builds it, .github/workflows/linux-x64-cpu-clang.yml:
    -DNCNN_AVX=OFF -DNCNN_AVX2=OFF, Release, clang
  • No sanitizer and no coverage instrumentation in the build that crashes
  • Linux x86-64

how to reproduce

  1. Save these four lines as crash.param:
7767517
2 2
Input                    in    0 1 in
MultiHeadAttention       mha   1 1 in out 0=64 1=0
  1. Load it:
#include "net.h"
int main(int argc, char** argv)
{
    ncnn::Net net;
    net.load_param(argv[1]);       // crash.param
    return 0;
}
  1. Floating point exception. Deterministic: 5 runs out of 5 on a plain Release build.

more

Root cause. MultiHeadAttention::load_param reads both operands from the param file
and checks neither:

int MultiHeadAttention::load_param(const ParamDict& pd)
{
    embed_dim = pd.get(0, 0);
    num_heads = pd.get(1, 1);
    ...
    scale = pd.get(6, 1.f / sqrtf(embed_dim / num_heads));   // line 59

embed_dim and num_heads are both int, so embed_dim / num_heads is an integer
division, and num_heads == 0 raises SIGFPE before sqrtf is ever called.

The default of 1 does not protect it. pd.get(1, 1) returns 1 only when key 1 is
absent. A param file that sets 1=0 supplies zero, and zero is what is used. Omitting
1= entirely does not crash - I checked, it loads cleanly.

Supplying scale does not protect it either. This is the part worth noting: the
expression 1.f / sqrtf(embed_dim / num_heads) is a C++ default argument, so it is
evaluated unconditionally at the call site, whether or not key 6 is present. I confirmed
this - ... 0=64 1=0 6=1.250000e-01 crashes identically.

Suggested fix, rejecting the value where it is read rather than at the division:

num_heads = pd.get(1, 1);
if (num_heads <= 0)
{
    NCNN_LOGE("MultiHeadAttention invalid num_heads %d", num_heads);
    return -1;
}

A negative num_heads is worth rejecting in the same check; it does not trap, but it
produces a negative head dimension that nothing downstream expects.

Same pattern, different layer. #6911 reports the same shape of defect in
ConvolutionDepthWise::load_param — an integer divide by a param value that is not
checked. This one is in MultiHeadAttention and is not covered by that report or by
#6913, #6914 or #6915, none of which touch multiheadattention.cpp. It may be worth
sweeping the other load_param implementations for the same construct while you are in
there.

Found by fuzzing the param reader. Happy to send more detail if it is useful.

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