Repository navigation
Fix out-of-bounds write in path mask border gap-filling - #21466
Merged
Merged
Conversation
Collaborator
|
Do you have any idea how that happend in your session? |
Collaborator
Author
LOL Not exactly. Something weird happened, and one edge of a mask was pulled very far away. I don't know how that happened, maybe a wrong gesture/key combo. I just saw for a fraction of a second an extremely stretched segment, and then darktable went kaput. I can't exclude that the weird thingy that exposed this bug was a bug in itself, but I don't know really. It was all very fast. |
Member
|
Needs a release note entry. TIA. |
1 task done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is arguably a rare latent bug that I was lucky enough to stumble upon in the middle of an editing session.
Fireworks ensued.
Problem
darktable crashes (
SIGSEGV, heap buffer overflow) while moving the mouse during path-mask creation. Backtrace:Root cause
_path_points_fill_border_gaps()builds an arc between two border points. When the next segment is collapsed — its corner and both control handles essentially coincident —_path_border_get_XYcomputes a zero derivative and returnsDT_INVALID_COORDINATE(-FLT_MAX) for the border pointbmax. From there:dt_fast_hypotf()on that point overflows the radius to+Inf.l = (a2 - a1) * fmaxf(r1, r2)becomesInf;(int)Infsaturates toINT_MAX.2*(l-1)overflowsintto a small negative value, sodt_masks_dynbuf_reserve_n()skips its buffer-growth check (and rewindspos) and returns a pointer into the unenlarged buffer.for(int i = 1; i < l; i++)then performs ~2.1 billion sequential writes, running off the end of the buffer into unmapped memory.Why it's rare
The crash needs
bmaxto be exactlyDT_INVALID_COORDINATE, i.e. an exactly-zero segment derivative. The caller deliberately samples the border att = 0.00001rather thant = 0precisely to avoid this: at that offset even an ordinary sharp-corner node (handle on the corner) still yields a small non-zero tangent, sobmaxis valid and the arc length stays small. Only a fully degenerate segment (corner ≈ both handles) makes the tangent round to zero. Such segments occur only transiently — e.g. dropping a node almost on top of another, or a node mid-creation before its handles spread — and only withnb >= 3, which gates this code path. Once the precondition is met the overflow fires deterministically; the rarity is entirely in reaching that degenerate geometry.Proposed fix
In
_path_points_fill_border_gaps():bmin/bmax/cmax) isDT_INVALID_COORDINATE. This compares against a finite sentinel, so it holds regardless of fast-math flags.INT_MAX/4before theintcast, so2*(l-1)can never overflow.Co-authored with Claude.