fix panic / nil pointer dereference on invalid patterns#9
Open
fix panic / nil pointer dereference on invalid patterns#9
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a panic in Pattern.match triggered by malformed-but-initially-validated patterns, by ensuring compile() doesn’t leave Pattern in a partially-initialized state and adding a defensive nil-check. Also expands CI to run lint/tests across platforms and Go release channels.
Changes:
- Prevent partial
matchTypeupdates on failed regexp compilation; add defense-in-depth check before usingp.regexp. - Add regression test covering repeated matching on a malformed pattern (no panic; consistent
ErrBadPattern). - Update GitHub Actions workflows to add concurrency cancellation and broaden the Go/platform matrix.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| patternmatcher.go | Fixes the panic by making compile() state updates atomic and guarding against nil regexp. |
| patternmatcher_test.go | Adds regression test ensuring malformed patterns don’t panic on repeated match calls. |
| .github/workflows/validate.yml | Updates lint workflow matrix/platform coverage and adds concurrency control. |
| .github/workflows/test.yml | Updates test workflow matrix/platform coverage and adds concurrency control. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`Pattern.compile` was updating `Pattern.matchType` in-place. In situations where
the resulting regex failed to compile, it would return early (with an error),
but the `matchType` was already set.
In that situation, `Pattern.match` would consider the `matchType` already
set, skip the `p.matchType == unknownMatch` condition, and fall through
to trying to use `p.regex`, which was nil, and resulted in a panic;
```
journalctl -u docker.service -f
dockerd[423967]: panic: runtime error: invalid memory address or nil pointer dereference
dockerd[423967]: [signal SIGSEGV: segmentation violation code=0x1 addr=0x90 pc=0x557e0f7ebf80]
dockerd[423967]: goroutine 1241 [running]:
dockerd[423967]: regexp.(*Regexp).doExecute(0x557e11b285a0?, {0x0?, 0x0?}, {0x0?, 0x557e11922650?, 0x557e11922650?}, {0xc0009d3db0?, 0xc000061778?}, 0x557e0f6d0d99?, 0x0, ...)
dockerd[423967]: /usr/local/go/src/regexp/exec.go:527 +0x80
dockerd[423967]: regexp.(*Regexp).doMatch(...)
dockerd[423967]: /usr/local/go/src/regexp/exec.go:514
dockerd[423967]: regexp.(*Regexp).MatchString(...)
dockerd[423967]: /usr/local/go/src/regexp/regexp.go:527
dockerd[423967]: github.com/moby/patternmatcher.(*Pattern).match(0x557e11922650?, {0xc0009d3db0, 0x1})
dockerd[423967]: /root/build-deb/engine/vendor/github.com/moby/patternmatcher/patternmatcher.go:334 +0x26b
dockerd[423967]: github.com/moby/patternmatcher.(*PatternMatcher).MatchesOrParentMatches(0xc000d761e0, {0xc0009d3db0, 0x1})
dockerd[423967]: /root/build-deb/engine/vendor/github.com/moby/patternmatcher/patternmatcher.go:142 +0xda
dockerd[423967]: github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb.validateCopySourcePath({0xc0009d3db0, 0x1}, 0xc0000621f8)
dockerd[423967]: /root/build-deb/engine/vendor/github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb/convert.go:2023 +0x55
dockerd[423967]: github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb.dispatchCopy(_, {{{0xc0009d3dc0, 0x1}, {0xc0009b5d10, 0x1, 0x1}, {0x0, 0x0, 0x0}}, {0x0, ...}, ...})
dockerd[423967]: /root/build-deb/engine/vendor/github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb/convert.go:1607 +0xd5c
dockerd[423967]: github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb.dispatch(_, {{_, _}, {_, _, _}, _}, {0xc000aab6a0, {0x557e1214c560, 0xc0007b79e0}, ...})
dockerd[423967]: /root/build-deb/engine/vendor/github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb/convert.go:1004 +0xafb
dockerd[423967]: github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb.toDispatchState({_, _}, {_, _, _}, {{0xc000d5c8d0, {0x0, 0x0}, {0x0, 0x0}, ...}, ...})
dockerd[423967]: /root/build-deb/engine/vendor/github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb/convert.go:731 +0x3926
dockerd[423967]: github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb.Dockerfile2LLB({_, _}, {_, _, _}, {{0xc000d5c8d0, {0x0, 0x0}, {0x0, 0x0}, ...}, ...})
dockerd[423967]: /root/build-deb/engine/vendor/github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb/convert.go:90 +0x65
dockerd[423967]: github.com/moby/buildkit/frontend/dockerfile/builder.Build.func6({0x557e121613e0, 0xc000cfd590}, 0x0, 0x557e0f64accb?)
dockerd[423967]: /root/build-deb/engine/vendor/github.com/moby/buildkit/frontend/dockerfile/builder/build.go:136 +0xfe
dockerd[423967]: github.com/moby/buildkit/frontend/dockerui.(*Client).Build.func1()
dockerd[423967]: /root/build-deb/engine/vendor/github.com/moby/buildkit/frontend/dockerui/build.go:39 +0x71
dockerd[423967]: golang.org/x/sync/errgroup.(*Group).Go.func1()
dockerd[423967]: /root/build-deb/engine/vendor/golang.org/x/sync/errgroup/errgroup.go:93 +0x50
dockerd[423967]: created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 1136
dockerd[423967]: /root/build-deb/engine/vendor/golang.org/x/sync/errgroup/errgroup.go:78 +0x95
systemd[1]: docker.service: Main process exited, code=exited, status=2/INVALIDARGUMENT
systemd[1]: docker.service: Failed with result 'exit-code'.
```
This patch:
- updates `Pattern.compile` to use a local variable for the intermediate
state, and only updates `Pattern.matchType` when completing successfully.
- adds a nil-check in `Pattern.match` as defense-in-depth.
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Member
Author
|
@tonistiigi @jsternberg PTAL 🤗 |
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.
fix panic / nil pointer dereference on invalid patterns
Pattern.compilewas updatingPattern.matchTypein-place. In situations wherethe resulting regex failed to compile, it would return early (with an error),
but the
matchTypewas already set.In that situation,
Pattern.matchwould consider thematchTypealreadyset, skip the
p.matchType == unknownMatchcondition, and fall throughto trying to use
p.regex, which was nil, and resulted in a panic;This patch:
Pattern.compileto use a local variable for the intermediatestate, and only updates
Pattern.matchTypewhen completing successfully.Pattern.matchas defense-in-depth.