Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't segfault when encountering permission denied errors #208

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

penguwin
Copy link
Member

Fixes #189

The bug was caused by accessing a nil value from ArchiveResult<Archive, Error> in order to get the Archive.Path when encountering errors during file collection (such as permission denied) for snapshot creation in snapshot.go.

I decided to always include a set Archive when sending a ArchiveResult into the channel in scanner.go. This is for the following reasons:

  • We need it to report to the user which file path exactly encountered issues during snapshot creation.
  • We don't want to return any other error value than nil or filepath/fs.SkipDir in our WalkFunk implementation for filepath.Walk as this will cause Walk to stop walking the entire tree. The behavior when to stop is defined via the --pedantic flag and set off by returning non-nil Error values in ArchiveResult.

This PR also fixes a another segfault when entering invalid exclude patterns such as -x "\\". knoxite will now immediately exit regardless of --pedantic with a message:

Failed to store '/tmp/knoxite-189/foo': Invalid exclude filter '\': syntax error in pattern

as the pattern will never match and is probably an unintended error/typo from the user.

@penguwin penguwin added this to the 1.0 milestone Nov 24, 2021
@penguwin penguwin requested a review from a team November 24, 2021 14:32
}

// if errors.Is(err, fs.ErrPermission) ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about those comments?

Copy link
Member Author

@penguwin penguwin Nov 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well.. 😅 I left them there on purpose to make it clear that we would also return nil for these error-cases. Can ofc remove them if desired

@penguwin penguwin requested a review from muesli March 24, 2023 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
First release
Awaiting triage
Development

Successfully merging this pull request may close these issues.

segmentation violation after encounter of (broken) symlink or permission denied
2 participants