Skip to content

Use Go 1.18's testing.F on simple fuzzers#7056

Merged
mxpv merged 3 commits intocontainerd:mainfrom
kzys:go118-fuzz
Jun 15, 2022
Merged

Use Go 1.18's testing.F on simple fuzzers#7056
mxpv merged 3 commits intocontainerd:mainfrom
kzys:go118-fuzz

Conversation

@kzys
Copy link
Copy Markdown
Member

@kzys kzys commented Jun 13, 2022

These two fuzzers are simple enough to use Go 1.18's testing.F

@k8s-ci-robot
Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kzys kzys marked this pull request as ready for review June 13, 2022 17:20
@kzys kzys force-pushed the go118-fuzz branch 2 times, most recently from c64378f to 086db70 Compare June 13, 2022 17:44
Copy link
Copy Markdown
Member

@mxpv mxpv left a 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

@kzys
Copy link
Copy Markdown
Member Author

kzys commented Jun 13, 2022

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.

@AdamKorcz
Copy link
Copy Markdown
Contributor

AdamKorcz commented Jun 13, 2022

Should we enable these on CI?
go test -fuzz FuzzXYZ

@AdamKorcz - Would it be covered by #7052?

No

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

Kazuyoshi Kato added 2 commits June 15, 2022 14:56
@kzys kzys force-pushed the go118-fuzz branch 2 times, most recently from 106201b to bf88348 Compare June 15, 2022 16:15
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 <[email protected]>
@kzys
Copy link
Copy Markdown
Member Author

kzys commented Jun 15, 2022

The last commit is adding simple go test -fuzz. While these tests are used by CI Fuzz, I'd like to make sure simple go test -fuzz is working as well.

@kzys
Copy link
Copy Markdown
Member Author

kzys commented Jun 15, 2022

@mxpv Can you take a look again? Your comment should be addressed by the last commit.

Copy link
Copy Markdown
Member

@mxpv mxpv left a comment

Choose a reason for hiding this comment

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

Thanks!

@mxpv mxpv merged commit 8aa3459 into containerd:main Jun 15, 2022
"github.com/containerd/containerd/pkg/cap"
)

func FuzzParseProcPIDStatus(data []byte) int {
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member Author

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.

Copy link
Copy Markdown
Member

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)?

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor

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 {
Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member Author

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.

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

Successfully merging this pull request may close these issues.

6 participants