Skip to content

fix: guard LinearModelSolver minimal fast-path against a null sample#52

Draft
f-dy wants to merge 1 commit into
danini:masterfrom
f-dy:fix/solver-linear-model-segfault
Draft

fix: guard LinearModelSolver minimal fast-path against a null sample#52
f-dy wants to merge 1 commit into
danini:masterfrom
f-dy:fix/solver-linear-model-segfault

Conversation

@f-dy

@f-dy f-dy commented May 31, 2026

Copy link
Copy Markdown

fix: guard LinearModelSolver minimal fast-path against a null sample

Problem

LinearModelSolver<2> / <3> (estimators/solver_linear_model.h) has a
minimal-sample fast-path in estimateModel:

if constexpr (_DimensionNumber == 2)
    if (sampleNumber_ == sampleSize())
        return estimate2DLine(data_, sample_, ...);   // derefs sample_[0..1]

if constexpr (_DimensionNumber == 3)
    if (sampleNumber_ == sampleSize())
        return estimate3DPlane(data_, sample_, ...);  // derefs sample_[0..2]

estimate2DLine/estimate3DPlane index sample_[0..k-1]. But estimateModel
explicitly supports sample_ == nullptr (meaning "use the first
sampleNumber_ points") — the general path right below it does exactly that via
const int sampleIdx = sample_ == nullptr ? pointIdx : sample_[pointIdx];.

So when the non-minimal fit is called with sample_ == nullptr and
sampleNumber_ == sampleSize(), the fast-path dereferences a null pointer and
crashes. This happens in MAGSAC's IRLS (sigmaConsensusPlusPlus), which refits
on all collected inliers passing sample_ = nullptr; when the inlier count is
exactly sampleSize() (3 for a plane, 2 for a line) it hits the fast-path.

Fix

Only take the minimal fast-path when the sample is non-null; otherwise fall
through to the general, null-safe path:

if (sampleNumber_ == sampleSize() && sample_ != nullptr)

Two-character change at each of the two sites. No behavior change for non-null
samples (the general path produces the same minimal-sample result).

Reproduction

Calling the non-minimal path directly with a null sample:

LinearModelSolver<3> solver;                 // 3D plane, sampleSize() == 3
solver.estimateModel(data, /*sample=*/nullptr, /*sampleNumber=*/3, models, nullptr);
  • Before: AddressSanitizer: SEGV on address 0x0 at
    estimate3DPlane (solver_linear_model.h).
  • After: returns normally (general path), no crash.

Scope

Standalone crash fix, branched from master, one file, zero coupling to the
gamma/DoF/scoring work. (It was previously bundled into the per-estimator-DoF
PR #50; it is extracted here so it
can merge independently.)

LinearModelSolver<2>/<3>::estimateModel has a minimal-sample fast-path that
calls estimate2DLine/estimate3DPlane(data_, sample_, ...), which dereferences
sample_[0..k-1]. But estimateModel explicitly supports sample_ == nullptr
(meaning "use the first sampleNumber_ points"), as the general path below
handles via 'sample_ == nullptr ? pointIdx : sample_[pointIdx]'.

When the non-minimal fit is called with sample_ == nullptr AND
sampleNumber_ == sampleSize() (e.g. MAGSAC's IRLS refitting on exactly
sampleSize() collected inliers — 3 for a plane, 2 for a line), the fast-path
dereferences the null pointer and crashes.

Fix: only take the fast-path when 'sample_ != nullptr'; otherwise fall through
to the general (null-safe) path. Two-character condition change, no behavior
change for non-null samples.

Verified with a direct repro: estimateModel(data, nullptr, sampleSize(), ...)
SEGVs at estimate3DPlane (read of address 0) before the fix and returns
normally after it.
@f-dy f-dy marked this pull request as ready for review May 31, 2026 22:39
@f-dy f-dy marked this pull request as draft May 31, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant