CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Use Go 1.18's testing.F on simple fuzzers #7056
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
Conversation
Skipping CI for Draft Pull Request. |
c64378f
to
086db70
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we enable these on CI?
go test -fuzz FuzzXYZ
@AdamKorcz - Would it be covered by #7052? I don't want to maintain the list of the fuzzing tests if possible. |
No |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
106201b
to
bf88348
Compare
In addition to oss-fuzz's CIFuzz (see containerd#7052), this commit adds a small shell script that run all fuzzing tests with go test -fuzz. While running for 30 seconds would be too short to acutally find issues, we want to make sure that these fuzzing tests are not fundamentally broken. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
The last commit is adding simple |
@mxpv Can you take a look again? Your comment should be addressed by the last commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
"github.com/containerd/containerd/pkg/cap" | ||
) | ||
|
||
func FuzzParseProcPIDStatus(data []byte) int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this removed? Is it not needed for the oss-fuzz integration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is moved under pkg/cap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the signature is now func (*testing.F)
rather than func ([]byte) int
and is moved into a _test.go
file. Does the external OSS-fuzz runner that consumes these functions (from #4841) know how to invoke a func (*testing.F)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the external OSS-fuzz runner that consumes these functions (from #4841) know how to invoke a
func (*testing.F)
?
Yes. Some other changes might be needed in the build script, though. For example, utilities from _test.go
files are currently not available to OSS-Fuzz, so these files need to be non _test.go
files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the signature is now func (*testing.F) rather than func ([]byte) int
Both can be used by OSS-Fuzz.
"github.com/containerd/containerd/platforms" | ||
) | ||
|
||
func FuzzPlatformsParse(data []byte) int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question on this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is moved under platforms/. Since Go's fuzzing support is part of testing package. I'm moving fuzzing from contrib/fuzz to each package if possible.
These two fuzzers are simple enough to use Go 1.18's testing.F