go
/
gvisor
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity
gvisor/nogo.yaml

190 lines
6.5 KiB
YAML
Raw Normal View History

Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
groups:
# We define three basic groups: generated (all generated files),
Add the gVisor admission webhook PiperOrigin-RevId: 339913577
2020-10-30 19:00:11 +00:00
# external (all files outside the repository), and internal (all
Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
# files within the local repository). We can't enforce many style
# checks on generated and external code, so enable those cases
# selectively for analyzers below.
- name: generated
regex: "^(bazel-genfiles|bazel-out|bazel-bin)/"
default: true
- name: external
regex: "^external/"
default: false
- name: internal
regex: ".*"
default: true
global:
generated:
suppress:
# Suppress the basic style checks for
# generated code, but keep the analysis
# that are required for quality & security.
- "should not use ALL_CAPS in Go names"
- "should not use underscores"
- "comment on exported"
- "methods on the same type should have the same receiver name"
- "at least one file in a package"
- "package comment should be of the form"
# Generated code may have dead code paths.
- "identical build constraints"
- "no value of type"
- "is never used"
# go_embed_data rules generate unicode literals.
- "string literal contains the Unicode format character"
- "string literal contains the Unicode control character"
Bump honnef.co/go/tools to v0.0.1-2020.1.6 PiperOrigin-RevId: 339476515
2020-10-28 16:38:17 +00:00
- "string literal contains Unicode control characters"
- "string literal contains Unicode format and control characters"
Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
# Some external code will generate protov1
# implementations. These should be ignored.
- "proto.* is deprecated"
- "xxx_messageInfo_.*"
- "receiver name should be a reflection of its identity"
# Generated gRPC code is not compliant either.
- "error strings should not be capitalized"
- "grpc.Errorf is deprecated"
Add the gVisor admission webhook PiperOrigin-RevId: 339913577
2020-10-30 19:00:11 +00:00
# Generated proto code does not always follow capitalization conventions.
- "(field|method|struct|type) .* should be .*"
# Generated proto code sometimes duplicates imports with aliases.
- "duplicate import"
Mix checklocks and atomic analyzers. This change makes the checklocks analyzer considerable more powerful, adding: * The ability to traverse complex structures, e.g. to have multiple nested fields as part of the annotation. * The ability to resolve simple anonymous functions and closures, and perform lock analysis across these invocations. This does not apply to closures that are passed elsewhere, since it is not possible to know the context in which they might be invoked. * The ability to annotate return values in addition to receivers and other parameters, with the same complex structures noted above. * Ignoring locking semantics for "fresh" objects, i.e. objects that are allocated in the local frame (typically a new-style function). * Sanity checking of locking state across block transitions and returns, to ensure that no unexpected locks are held. Note that initially, most of these findings are excluded by a comprehensive nogo.yaml. The findings that are included are fundamental lock violations. The changes here should be relatively low risk, minor refactorings to either include necessary annotations to simplify the code structure (in general removing closures in favor of methods) so that the analyzer can be easily track the lock state. This change additional includes two changes to nogo itself: * Sanity checking of all types to ensure that the binary and ast-derived types have a consistent objectpath, to prevent the bug above from occurring silently (and causing much confusion). This also requires a trick in order to ensure that serialized facts are consumable downstream. This can be removed with https://go-review.googlesource.com/c/tools/+/331789 merged. * A minor refactoring to isolation the objdump settings in its own package. This was originally used to implement the sanity check above, but this information is now being passed another way. The minor refactor is preserved however, since it cleans up the code slightly and is minimal risk. PiperOrigin-RevId: 382613300
2021-07-01 22:05:28 +00:00
# These will never be annotated.
- "unexpected call to atomic function"
Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
internal:
suppress:
# We use ALL_CAPS for system definitions,
# which are common enough in the code base
# that we shouldn't annotate exceptions.
#
# Same story for underscores.
- "should not use ALL_CAPS in Go names"
- "should not use underscores in Go names"
Mix checklocks and atomic analyzers. This change makes the checklocks analyzer considerable more powerful, adding: * The ability to traverse complex structures, e.g. to have multiple nested fields as part of the annotation. * The ability to resolve simple anonymous functions and closures, and perform lock analysis across these invocations. This does not apply to closures that are passed elsewhere, since it is not possible to know the context in which they might be invoked. * The ability to annotate return values in addition to receivers and other parameters, with the same complex structures noted above. * Ignoring locking semantics for "fresh" objects, i.e. objects that are allocated in the local frame (typically a new-style function). * Sanity checking of locking state across block transitions and returns, to ensure that no unexpected locks are held. Note that initially, most of these findings are excluded by a comprehensive nogo.yaml. The findings that are included are fundamental lock violations. The changes here should be relatively low risk, minor refactorings to either include necessary annotations to simplify the code structure (in general removing closures in favor of methods) so that the analyzer can be easily track the lock state. This change additional includes two changes to nogo itself: * Sanity checking of all types to ensure that the binary and ast-derived types have a consistent objectpath, to prevent the bug above from occurring silently (and causing much confusion). This also requires a trick in order to ensure that serialized facts are consumable downstream. This can be removed with https://go-review.googlesource.com/c/tools/+/331789 merged. * A minor refactoring to isolation the objdump settings in its own package. This was originally used to implement the sanity check above, but this information is now being passed another way. The minor refactor is preserved however, since it cleans up the code slightly and is minimal risk. PiperOrigin-RevId: 382613300
2021-07-01 22:05:28 +00:00
# These need to be annotated.
- "unexpected call to atomic function.*"
- "return with unexpected locks held.*"
- "incompatible return states.*"
Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
exclude:
Remove existing nogo exceptions. PiperOrigin-RevId: 347047550
2020-12-11 20:04:46 +00:00
# Generated: exempt all.
Delete shim v1 gvisor-containerd-shim is not compatible with containerd 1.1 or earlier. Starting from containerd 1.2, shim v2 is the preferred interface. PiperOrigin-RevId: 351485556
2021-01-13 01:50:33 +00:00
- pkg/shim/runtimeoptions/runtimeoptions_cri.go
Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
analyzers:
asmdecl:
external: # Enabled.
assign:
external:
exclude:
- gazelle/walk/walk.go
atomic:
external: # Enabled.
bools:
external: # Enabled.
buildtag:
external: # Enabled.
cgocall:
external: # Enabled.
Add checklocks analyzer. This validates that struct fields if annotated with "// checklocks:mu" where "mu" is a mutex field in the same struct then access to the field is only done with "mu" locked. All types that are guarded by a mutex must be annotated with // +checklocks:<mutex field name> For more details please refer to README.md. PiperOrigin-RevId: 360729328
2021-03-03 20:18:04 +00:00
checklocks:
internal:
exclude:
Mix checklocks and atomic analyzers. This change makes the checklocks analyzer considerable more powerful, adding: * The ability to traverse complex structures, e.g. to have multiple nested fields as part of the annotation. * The ability to resolve simple anonymous functions and closures, and perform lock analysis across these invocations. This does not apply to closures that are passed elsewhere, since it is not possible to know the context in which they might be invoked. * The ability to annotate return values in addition to receivers and other parameters, with the same complex structures noted above. * Ignoring locking semantics for "fresh" objects, i.e. objects that are allocated in the local frame (typically a new-style function). * Sanity checking of locking state across block transitions and returns, to ensure that no unexpected locks are held. Note that initially, most of these findings are excluded by a comprehensive nogo.yaml. The findings that are included are fundamental lock violations. The changes here should be relatively low risk, minor refactorings to either include necessary annotations to simplify the code structure (in general removing closures in favor of methods) so that the analyzer can be easily track the lock state. This change additional includes two changes to nogo itself: * Sanity checking of all types to ensure that the binary and ast-derived types have a consistent objectpath, to prevent the bug above from occurring silently (and causing much confusion). This also requires a trick in order to ensure that serialized facts are consumable downstream. This can be removed with https://go-review.googlesource.com/c/tools/+/331789 merged. * A minor refactoring to isolation the objdump settings in its own package. This was originally used to implement the sanity check above, but this information is now being passed another way. The minor refactor is preserved however, since it cleans up the code slightly and is minimal risk. PiperOrigin-RevId: 382613300
2021-07-01 22:05:28 +00:00
- "^-$" # b/181776900: analyzer fails on buildkite.
Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
shadow: # Disable for now.
generated:
exclude: [".*"]
internal:
exclude: [".*"]
composites: # Disable for now.
generated:
exclude: [".*"]
internal:
exclude: [".*"]
errorsas:
external: # Enabled.
httpresponse:
external: # Enabled.
loopclosure:
external: # Enabled.
nilfunc:
external: # Enabled.
nilness:
internal:
exclude:
- pkg/sentry/platform/kvm/kvm_test.go # Intentional.
- tools/bigquery/bigquery.go # False positive.
printf:
external: # Enabled.
shift:
Add //pkg/sync:generic_atomicptrmap. AtomicPtrMap is a generic concurrent map from arbitrary keys to arbitrary pointer values. Benchmarks: name time/op StoreDelete/RWMutexMap-12 335ns ± 1% StoreDelete/SyncMap-12 705ns ± 3% StoreDelete/AtomicPtrMap-12 287ns ± 4% StoreDelete/AtomicPtrMapSharded-12 289ns ± 1% LoadOrStoreDelete/RWMutexMap-12 342ns ± 2% LoadOrStoreDelete/SyncMap-12 662ns ± 2% LoadOrStoreDelete/AtomicPtrMap-12 290ns ± 7% LoadOrStoreDelete/AtomicPtrMapSharded-12 293ns ± 2% LookupPositive/RWMutexMap-12 101ns ±26% LookupPositive/SyncMap-12 202ns ± 2% LookupPositive/AtomicPtrMap-12 71.1ns ± 2% LookupPositive/AtomicPtrMapSharded-12 73.2ns ± 1% LookupNegative/RWMutexMap-12 119ns ± 1% LookupNegative/SyncMap-12 154ns ± 1% LookupNegative/AtomicPtrMap-12 84.7ns ± 3% LookupNegative/AtomicPtrMapSharded-12 86.8ns ± 1% Concurrent/FixedKeys_1PercentWrites_RWMutexMap-12 1.32µs ± 2% Concurrent/FixedKeys_1PercentWrites_SyncMap-12 52.7ns ±10% Concurrent/FixedKeys_1PercentWrites_AtomicPtrMap-12 31.8ns ±20% Concurrent/FixedKeys_1PercentWrites_AtomicPtrMapSharded-12 24.0ns ±15% Concurrent/FixedKeys_10PercentWrites_RWMutexMap-12 860ns ± 3% Concurrent/FixedKeys_10PercentWrites_SyncMap-12 68.8ns ±20% Concurrent/FixedKeys_10PercentWrites_AtomicPtrMap-12 98.6ns ± 7% Concurrent/FixedKeys_10PercentWrites_AtomicPtrMapSharded-12 42.0ns ±25% Concurrent/FixedKeys_50PercentWrites_RWMutexMap-12 1.17µs ± 3% Concurrent/FixedKeys_50PercentWrites_SyncMap-12 136ns ±34% Concurrent/FixedKeys_50PercentWrites_AtomicPtrMap-12 286ns ± 3% Concurrent/FixedKeys_50PercentWrites_AtomicPtrMapSharded-12 115ns ±35% Concurrent/ChangingKeys_1PercentWrites_RWMutexMap-12 1.27µs ± 2% Concurrent/ChangingKeys_1PercentWrites_SyncMap-12 5.01µs ± 3% Concurrent/ChangingKeys_1PercentWrites_AtomicPtrMap-12 38.1ns ± 3% Concurrent/ChangingKeys_1PercentWrites_AtomicPtrMapSharded-12 22.6ns ± 2% Concurrent/ChangingKeys_10PercentWrites_RWMutexMap-12 1.08µs ± 2% Concurrent/ChangingKeys_10PercentWrites_SyncMap-12 5.97µs ± 1% Concurrent/ChangingKeys_10PercentWrites_AtomicPtrMap-12 390ns ± 2% Concurrent/ChangingKeys_10PercentWrites_AtomicPtrMapSharded-12 93.6ns ± 1% Concurrent/ChangingKeys_50PercentWrites_RWMutexMap-12 1.77µs ± 2% Concurrent/ChangingKeys_50PercentWrites_SyncMap-12 8.07µs ± 2% Concurrent/ChangingKeys_50PercentWrites_AtomicPtrMap-12 1.61µs ± 2% Concurrent/ChangingKeys_50PercentWrites_AtomicPtrMapSharded-12 386ns ± 1% Updates #231 PiperOrigin-RevId: 346614776
2020-12-09 20:43:56 +00:00
generated: # Disabled for generated code; these shifts are well-defined.
exclude: [".*"]
Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
external: # Enabled.
stringintconv:
external:
exclude:
- ".*protobuf/.*.go" # Bad conversions.
- ".*flate/huffman_bit_writer.go" # Bad conversion.
# Runtime internal violations.
- ".*reflect/value.go"
- ".*encoding/xml/xml.go"
- ".*runtime/pprof/internal/profile/proto.go"
- ".*fmt/scan.go"
- ".*go/types/conversions.go"
- ".*golang.org/x/net/dns/dnsmessage/message.go"
tests:
external: # Enabled.
unmarshal:
external: # Enabled.
unreachable:
external: # Enabled.
Fix per-analyzer overrides of default-disabled groups Currently behavior of config groups with `default: false` is buggy. The intention is that adding an empty suppression section for that group to a specific analyzer config should enable reporting for that analyzer. i.e., ``` groups: - name: foo regex: "^foo/" default: false global: ... analyzers: asmdecl: foo: # Enabled. ``` This should enable the foo group only for asmdecl. Unfortunately, today the actual behavior depends on the contents of the `global:` section. If `global:` contains an entry for foo, then it will work as described. If `global:` does _not_ contain an entry for foo, then the group default (disabled) always applies and the individual analyzer options have no effect. The cause of this is confusion in `AnalyzerConfig.shouldReport`, which doesn't distinguish between explicit suppression via a global suppression/exclude and simply having no configuration at all. Make this more explicit, so that the no configuration case can continue to per-analyzer configuration before falling back to the group default. The last test case in the added test fails without this change. This re-enables several opted-in analyzers for external dependencies, which have gained a few more false positives to suppress. PiperOrigin-RevId: 386904725
2021-07-26 17:24:11 +00:00
exclude:
- ".*protobuf/.*.go"
Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
unsafeptr:
internal:
exclude:
- ".*_test.go" # Exclude tests.
- "pkg/flipcall/.*_unsafe.go" # Special case.
- pkg/gohacks/gohacks_unsafe.go # Special case.
Move ring0 package. This allows the package to serve as a general purpose ring0 support package, as opposed to being bound to specific sentry platforms. Updates #5039 PiperOrigin-RevId: 355220044
2021-02-02 20:01:12 +00:00
- pkg/ring0/pagetables/allocator_unsafe.go # Special case.
Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
- pkg/sentry/fs/fsutil/host_file_mapper_unsafe.go # Special case.
- pkg/sentry/platform/kvm/bluepill_unsafe.go # Special case.
- pkg/sentry/platform/kvm/machine_unsafe.go # Special case.
- pkg/sentry/platform/safecopy/safecopy_unsafe.go # Special case.
Use memutil.MapFile for the memory accounting page. PiperOrigin-RevId: 381145216
2021-06-24 00:00:58 +00:00
- pkg/sentry/usage/memory_unsafe.go # Special case.
Add nogo configuration. This splits the nogo rules into a separate configuration yaml file, and allows for multiple files to be provided. Because attrs cannot be passed down to aspects, this required that all findings are propagated up the aspect Provider. This doesn't mean that any extra work must be done, just that this information must be carried through the graph, and some additional starlark complexity is required. PiperOrigin-RevId: 339076357
2020-10-26 18:09:34 +00:00
- pkg/sentry/vfs/mount_unsafe.go # Special case.
- pkg/state/decode_unsafe.go # Special case.
unusedresult:
external: # Enabled.
checkescape:
external: # Enabled.
checklinkname: rudimentary type-checking of linkname directives This CL introduces a 'checklinkname' analyzer, which provides rudimentary type-checking that verifies that function signatures on the local and remote sides of //go:linkname directives match expected values. If the Go standard library changes the definitions of any of these function, checklinkname will flag the change as a finding, providing an error informing the gVisor team to adapt to the upstream changes. This allows us to eliminate the majority of gVisor's forward-looking negative build tags, as we can catch mismatches in testing [1]. The remaining forward-looking negative build tags are covering shared struct definitions, which I hope to add to checklinkname in a future CL. [1] Of course, semantics/requirements can change without the signature changing, so we still must be careful, but this covers the common case. PiperOrigin-RevId: 387873847
2021-07-30 20:39:18 +00:00
checklinkname:
external: # Enabled.
suppress:
# We don't care to check every single linkname in the Go standard
# library. Suppress findings about stdlib linkname targets we haven't
# described in checklinkname.
#
# Note that we _do_ want to check the signature of the known linkname
# targets in the standard library, so we still need to run
# checklinkname on stdlib generally.
- "linkname to unknown symbol"
Remove existing nogo exceptions. PiperOrigin-RevId: 347047550
2020-12-11 20:04:46 +00:00
SA4016:
internal:
exclude:
- pkg/gohacks/gohacks_unsafe.go # x ^ 0 always equals x.
SA2001:
internal:
exclude:
- pkg/sentry/fs/fs.go # Intentional.
- pkg/sentry/fs/gofer/inode.go # Intentional.
- pkg/refs/refcounter_test.go # Intentional.
Suppress ST1019 on generated protobufs PiperOrigin-RevId: 355915778
2021-02-05 21:26:22 +00:00
ST1019:
generated:
exclude:
# package ".../kubeapi/core/v1/v1" is being imported more than once
- generated.gen.pb.go
Remove existing nogo exceptions. PiperOrigin-RevId: 347047550
2020-12-11 20:04:46 +00:00
ST1021:
internal:
suppress:
- "comment on exported type Translation" # Intentional.
- "comment on exported type PinnedRange" # Intentional.
SA5011:
internal:
exclude:
Update go-tools to 2020.2.1 ...and a bunch of other things as I worked through the rot. Notably: - Upgrade to bazel 4.0.0 - Upgrade to Go 1.15.7 Remove go_branch stderr suppression; this made it quite difficult to see what was failing while developing this patch. PiperOrigin-RevId: 355257833
2021-02-02 22:49:55 +00:00
# https://github.com/dominikh/go-tools/issues/924
- pkg/sentry/fs/fdpipe/pipe_opener_test.go
- pkg/tcpip/tests/integration/link_resolution_test.go