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

Deal with CopySpecs that include kernel file system directories better #71

Closed
bmr-cymru opened this issue Nov 29, 2012 · 4 comments
Closed
Assignees
Milestone

Comments

@bmr-cymru
Copy link
Member

Currently scooping up a whole directory tree from /sys or /proc can trigger lots of ugly errors in -v mode. There's also an associated problem of triggering dmesg warnings when we touch deprecated files (e.g. several ipv6 sysctls).

There's a few ways we could deal with this:

  • Check read permissions, skip files with --w--w--w-.and truncate/chmod instead
  • Add an additional parameter to suppress all IO errors
  • Add some kind of path black/whitelisting

The last is probably a more maintainable way to cope with these problems long-term but in the short term the others are much easier to implement with our current structure.

@jhjaggars
Copy link
Contributor

Can we not use the addForbiddenPath method to avoid these paths?

@bmr-cymru
Copy link
Member Author

Yes - that's what I've been doing for e.g. the deprecated sysctls, see commit 8db01b2 on rhel-6 branch. The problem with this approach is it gets hairy when you have a large number of such paths to manage. For now it may be the best way forward but there are a number of deprecated paths to avoid in proc and a much larger set of paths that we shouldn't attempt to copy in sysfs - sys is ten years old now and we really should up our game in terms of the information we collect from there. Right now we have bigger problems (our recursive copy function isn't safe for trees containing looping symlinks for e.g.) but this is something that I think we need to improve.

bmr-cymru added a commit that referenced this issue Nov 30, 2012
Symlinks need special treatment when we're copying them into the
report. The target path stored in the report must always be
relative but we need to pass an absolute path for the target to
the recursive doCopyFileOrDir() call that picks up the target for
us.

There are further problems with the current code but these cannot
be fixed trivially: symbolic links to directories are currently
ignored but are fundamental to the layout of file systems like sys
(and to a lesser extent proc). Supporting this properly requires
an algorithm that can cope with arbitrary symlink loops in the
tree being copied and that correctly copies-in any links that may
point outside of the tree currently being copied.

See Issues #71, #72 for more details.
@ghost ghost assigned bmr-cymru Apr 25, 2013
@ghost ghost assigned adam-stokes and bmr-cymru Oct 31, 2013
@bmr-cymru bmr-cymru modified the milestone: 3.2 Apr 17, 2014
@bmr-cymru
Copy link
Member Author

This is now about 90% fixed. We now handle symlinks that point to directories (but we don't copy the target by default - that way lies madness) and block and character device nodes and other special node types are now handled properly. This means it's now safe to do e.g.:

        self.add_copy_spec("/sys")
        self.add_copy_spec("/dev")
        self.add_copy_spec("/proc")

There's still some work required to make this robust; in particular scooping up the whole of /sys and /proc will require the global blacklists (Issue #316) to avoid tripping over various pipes and comms channels that hang on read() IO.

@adam-stokes adam-stokes modified the milestones: 3.2, 3.3 Aug 19, 2014
@bmr-cymru
Copy link
Member Author

The fix for Issue #266 means that we handle unreadable files correctly. I'll do some more testing on this today and close the issue out if there's nothing more to do.

ddstreet pushed a commit to ddstreet/sos that referenced this issue Apr 15, 2016
Symlinks need special treatment when we're copying them into the
report. The target path stored in the report must always be
relative but we need to pass an absolute path for the target to
the recursive doCopyFileOrDir() call that picks up the target for
us.

There are further problems with the current code but these cannot
be fixed trivially: symbolic links to directories are currently
ignored but are fundamental to the layout of file systems like sys
(and to a lesser extent proc). Supporting this properly requires
an algorithm that can cope with arbitrary symlink loops in the
tree being copied and that correctly copies-in any links that may
point outside of the tree currently being copied.

See Issues sosreport#71, sosreport#72 for more details.
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

No branches or pull requests

3 participants