Details

Reviewers

asomers
rwatson
gnn

Commits

rS339095: MFC r336875:
rS336875: audit(4): add tests for sysctl(3) and sysarch(2)

Summary

This revision introduces a test-program for miscellaneous (or 'other') audit class.
The syscalls for which tests are introduced, are:

sysctl(3)
sysarch(2)

Note: Since sysarch(3) is dependent on machine architecture, tests will be generated
on the basis of allowed archs.

For example, on my machine, both AMD64 and i386 are supported, thus

▶ kyua list miscellaneous
miscellaneous:sysarch_amd64_success
miscellaneous:sysarch_failure
miscellaneous:sysarch_i386_success
miscellaneous:sysctl_failure
miscellaneous:sysctl_success

Test Plan

Execute make && make install from test/sys/audit.
Execute kyua test from /usr/tests/sys/audit. All testcases should succeed.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 18034
Build 17784: arc lint + arc unit

Event Timeline

aniketp created this revision.Jul 3 2018, 7:15 PM

Harbormaster completed remote builds in B17822: Diff 44824.Jul 3 2018, 7:15 PM

Updates in Copyright

Harbormaster completed remote builds in B17823: Diff 44826.Jul 3 2018, 7:24 PM

How about audit(2)?

Add test for audit(2) in failure case

Harbormaster completed remote builds in B17839: Diff 44856.Jul 4 2018, 3:09 PM

The failure test case looks ok, but I think we should wait for the success test case before committing.

asomers added inline comments.Jul 10 2018, 2:59 PM

tests/sys/audit/miscellaneous.c
261	I think you should rename the sysarch tests. We aren't actually testing `AMD64_GET_FSBASE`; we don't care what it does. We're just testing the architecture-independent audit(4) connection of sysarch(2). Therefore I think you should have just a single test case called `sysarch_success`. Use conditional compilation to call `atf_skip` on unsupported architectures.

Create a single success mode test case for sysarch(2) with sysnum being dependent on the system architecture

Harbormaster completed remote builds in B18030: Diff 45280.Jul 14 2018, 12:08 PM

asomers accepted this revision.Jul 14 2018, 2:58 PM

asomers added inline comments.

tests/sys/audit/miscellaneous.c
100	I'm curious. Why is sysarch unsafe on sparc?

This revision is now accepted and ready to land.Jul 14 2018, 2:58 PM

aniketp added inline comments.Jul 14 2018, 3:02 PM

tests/sys/audit/miscellaneous.c
100	Well. TBH, I don't know. The only allowed syscalls were `SPARC_UTRAP_INSTALL` and `SPARC_SIGTRAMP_INSTALL`. I don't know what they do and not sure if I should add them to the tests. Though if you think they are safe enough, I can add their tests no issue.

asomers added inline comments.Jul 14 2018, 3:40 PM

tests/sys/audit/miscellaneous.c
100	It looks fine. According to an old Solaris man page for that function, you can use it to install no handler for a certain trap. That should be a nop. https://docs.oracle.com/cd/E19455-01/806-0626/6j9vgh69s/index.html

Include Sparc64 in the possible architectures for sysarch(2)

This revision now requires review to proceed.Jul 14 2018, 4:34 PM

Harbormaster completed remote builds in B18033: Diff 45286.Jul 14 2018, 4:34 PM

Add the missed \#endif statement

Harbormaster completed remote builds in B18034: Diff 45287.Jul 14 2018, 4:39 PM

Add the missed sysnum for sparc64 tests

Harbormaster completed remote builds in B18035: Diff 45288.Jul 14 2018, 4:46 PM

asomers requested changes to this revision.Jul 15 2018, 8:57 PM

asomers added inline comments.

tests/sys/audit/miscellaneous.c
124	This isn't safe. For one thing, `sysarg` is uninitialized. For another, the kernel may be interpreting it differently for different syscalls. It might be expecting a pointer to a structure containing other pointers, for example.

This revision now requires changes to proceed.Jul 15 2018, 8:57 PM

Use respective args instead of a common void *sysarg

Harbormaster completed remote builds in B18088: Diff 45396.Jul 16 2018, 10:37 PM

asomers requested changes to this revision.Jul 16 2018, 11:10 PM

asomers added inline comments.

tests/sys/audit/miscellaneous.c
94	Technically this is an input/output argument, so you still need to initialize all fields or tools like Coverity will complain.
98	This should be a `register_t`, not a `void*`.
104	Did you mean to change the value of `sysnum`?
134	May as well inline sysnum here instead of setting it as a variable above.

This revision now requires changes to proceed.Jul 16 2018, 11:10 PM

aniketp marked 9 inline comments as done.Jul 17 2018, 11:00 AM

Changes as suggested in the previous review

Harbormaster completed remote builds in B18094: Diff 45407.Jul 17 2018, 11:00 AM

asomers accepted this revision.Jul 17 2018, 2:06 PM

This revision is now accepted and ready to land.Jul 17 2018, 2:06 PM

The build fails on arm.

/home/asomers/freebsd/base/head/tests/sys/audit/miscellaneous.c:99:39: error: expected ';' at end of declaration
        struct arm_sync_icache_args armsysarg {

This revision now requires changes to proceed.Jul 17 2018, 2:45 PM

aniketp added inline comments.Jul 17 2018, 3:00 PM

tests/sys/audit/miscellaneous.c
103	@asomers, I have the semicolon here. Why would the build fail otherwise?

Replace ARM_SYNC_ICACHE with ARM_DRAIN_WRITEBUF as the syscall argument

ARM_SYNC_ICACHE for some reason breaks build on arm as pointed out by @asomers. It can't find the declaration of the corresponding struct and hence, assumes that the test-program is declaring the struct instead of defining it.

Harbormaster completed remote builds in B18102: Diff 45424.Jul 17 2018, 7:24 PM

In D16116#346010, @aniketp wrote:

Replace ARM_SYNC_ICACHE with ARM_DRAIN_WRITEBUF as the syscall argument

ARM_SYNC_ICACHE for some reason breaks build on arm as pointed out by @asomers. It can't find the declaration of the corresponding struct and hence, assumes that the test-program is declaring the struct instead of defining it.

You should really try to figure out why it didn't compile with ARM_SYNC_ICACHE. It would be a good exercise for you.

Fix build for ARM and Sparc64 architectures

Harbormaster completed remote builds in B18169: Diff 45545.Jul 19 2018, 4:07 PM

Build fails on sparc.

/home/asomers/freebsd/base/head/tests/sys/audit/miscellaneous.c: In function 'atfu_sysarch_success_body':
/home/asomers/freebsd/base/head/tests/sys/audit/miscellaneous.c:106: warning: initialization makes pointer from integer without a cast
/home/asomers/freebsd/base/head/tests/sys/audit/miscellaneous.c:107: warning: initialization makes pointer from integer without a cast

This revision now requires changes to proceed.Jul 19 2018, 4:19 PM