Skip to content

Fix link cycle detection#158

Merged
wagoodman merged 7 commits intomainfrom
sym-link-detection-cycle-alex
Feb 21, 2023
Merged

Fix link cycle detection#158
wagoodman merged 7 commits intomainfrom
sym-link-detection-cycle-alex

Conversation

@wagoodman
Copy link
Copy Markdown
Contributor

@wagoodman wagoodman commented Feb 21, 2023 •
edited
Loading

PR #154 added a regression where link resolution may result in falsely claiming that there is a link cycle for links that reference the same path twice while resolving links for a single path.

For example:

# links:
   /usr/local/bin/ksh -> /bin/ksh
   /bin -> /usr/bin
   /etc/alternatives/ksh -> /bin/ksh93

# real file: 
   /usr/bin/ksh93

In this case while resolving links in the /usr/local/bin/ksh path, the /usr/bin path is resolved twice, which under the current (buggy) implementation will falsely claim that there is a cycle relating to /usr/bin.

Relates to anchore/syft#1586

spiffcs and others added 7 commits February 21, 2023 11:04
@spiffcs
test: add failing test for cycle case
f3c5c79 
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs
test: test updates sym links
3526f21 
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@wagoodman
change the filetree recursive pathset to represent open calls
74bead2 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman
add another cycle test
c396e5a 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman
change filetree attempting path set to counters
33b9c97 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman
remove comment
4725d6a 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman
fix linting
1f8e0c9 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman wagoodman marked this pull request as ready for review February 21, 2023 22:13
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 21, 2023

Benchmark Test Results

Benchmark results from the latest changes vs base branch 
latest: Pulling from library/ubuntu
goos: linux
goarch: amd64
pkg: github.com/anchore/stereoscope/pkg/file
cpu: Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz
docker: 
           │ ./.tmp/benchmark-5bbb589.txt │
           │            sec/op            │
TarIndex-2                   41.94µ ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

           │ ./.tmp/benchmark-5bbb589.txt │
           │             B/op             │
TarIndex-2                  5.560Ki ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

           │ ./.tmp/benchmark-5bbb589.txt │
           │          allocs/op           │
TarIndex-2                    93.00 ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

pkg: github.com/anchore/stereoscope/test/integration
                                      │ ./.tmp/benchmark-5bbb589.txt │
                                      │            sec/op            │
SimpleImage_GetImage/docker-archive-2                   1.522m ± ∞ ¹
SimpleImage_GetImage/oci-archive-2                      1.222m ± ∞ ¹
SimpleImage_GetImage/oci-dir-2                          805.5µ ± ∞ ¹
geomean                                                 1.144m
¹ need >= 6 samples for confidence interval at level 0.95

                                      │ ./.tmp/benchmark-5bbb589.txt │
                                      │             B/op             │
SimpleImage_GetImage/docker-archive-2                  354.0Ki ± ∞ ¹
SimpleImage_GetImage/oci-archive-2                     632.6Ki ± ∞ ¹
SimpleImage_GetImage/oci-dir-2                         399.8Ki ± ∞ ¹
geomean                                                447.4Ki
¹ need >= 6 samples for confidence interval at level 0.95

                                      │ ./.tmp/benchmark-5bbb589.txt │
                                      │          allocs/op           │
SimpleImage_GetImage/docker-archive-2                   2.732k ± ∞ ¹
SimpleImage_GetImage/oci-archive-2                      1.550k ± ∞ ¹
SimpleImage_GetImage/oci-dir-2                          1.334k ± ∞ ¹
geomean                                                 1.781k
¹ need >= 6 samples for confidence interval at level 0.95

docker: Error response from daemon: Get "http://localhost/v2/": dial tcp [::1]:80: connect: connection refused.
                                                   │ ./.tmp/benchmark-5bbb589.txt │
                                                   │            sec/op            │
SimpleImage_FetchSquashedContents/docker-archive-2                   17.13µ ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

                                                   │ ./.tmp/benchmark-5bbb589.txt │
                                                   │             B/op             │
SimpleImage_FetchSquashedContents/docker-archive-2                  2.648Ki ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

                                                   │ ./.tmp/benchmark-5bbb589.txt │
                                                   │          allocs/op           │
SimpleImage_FetchSquashedContents/docker-archive-2                    21.00 ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

kzantow
kzantow approved these changes Feb 21, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

👍

@spiffcs spiffcs mentioned this pull request Feb 21, 2023
Merged
@wagoodman wagoodman merged commit 529924d into main Feb 21, 2023
@wagoodman wagoodman deleted the sym-link-detection-cycle-alex branch February 21, 2023 22:49
@wagoodman wagoodman mentioned this pull request Feb 22, 2023
Merged
gnmahanth pushed a commit to deepfence/stereoscope that referenced this pull request Jun 15, 2023
@wagoodman @spiffcs @gnmahanth
Fix link cycle detection (anchore#158)
66b650a 
* test: add failing test for cycle case

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

* test: test updates sym links

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

* change the filetree recursive pathset to represent open calls

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* add another cycle test

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* change filetree attempting path set to counters

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* remove comment

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* fix linting

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

---------

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Co-authored-by: Christopher Phillips <christopher.phillips@anchore.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kzantow kzantow kzantow approved these changes

@spiffcs spiffcs spiffcs approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wagoodman @kzantow @spiffcs