Page MenuHomeFreeBSD
Differential D16116

Introduce tests for sysctl(3), and sysarch(2) for AMD64, i386, ARM & MIPS architectures
ClosedPublic
Actions

Authored by aniketp on Jul 3 2018, 7:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 15, 10:58 AM
Unknown Object (File)
Fri, Nov 15, 9:45 AM
Unknown Object (File)
Tue, Nov 12, 8:57 PM
Unknown Object (File)
Wed, Nov 6, 10:42 AM
Unknown Object (File)
Tue, Nov 5, 9:12 AM
Unknown Object (File)
Mon, Nov 4, 9:29 AM
Unknown Object (File)
Sat, Nov 2, 12:46 AM
Unknown Object (File)
Mon, Oct 28, 10:29 AM
View All Files
Subscribers
imp

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 18174
Build 17908: 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
aniketp updated this revision to Diff 44826.Jul 3 2018, 7:24 PM
  • Updates in Copyright
Harbormaster completed remote builds in B17823: Diff 44826.Jul 3 2018, 7:24 PM
asomers added a comment.Jul 3 2018, 7:29 PM

How about audit(2)?

aniketp updated this revision to Diff 44856.Jul 4 2018, 3:09 PM
  • Add test for audit(2) in failure case
Harbormaster completed remote builds in B17839: Diff 44856.Jul 4 2018, 3:09 PM
asomers added a comment.Jul 4 2018, 6:33 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.

aniketp updated this revision to Diff 45280.Jul 14 2018, 12:08 PM

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

aniketp updated this revision to Diff 45286.Jul 14 2018, 4:34 PM

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
aniketp updated this revision to Diff 45287.Jul 14 2018, 4:39 PM

Add the missed \#endif statement

Harbormaster completed remote builds in B18034: Diff 45287.Jul 14 2018, 4:39 PM
aniketp updated this revision to Diff 45288.Jul 14 2018, 4:46 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
aniketp updated this revision to Diff 45396.Jul 16 2018, 10:37 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
aniketp updated this revision to Diff 45407.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
asomers requested changes to this revision.Jul 17 2018, 2:45 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?

aniketp updated this revision to Diff 45424.EditedJul 17 2018, 7:24 PM
  • 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
asomers added a comment.Jul 17 2018, 8:14 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.

aniketp updated this revision to Diff 45545.Jul 19 2018, 4:07 PM
  • Fix build for ARM and Sparc64 architectures
Harbormaster completed remote builds in B18169: Diff 45545.Jul 19 2018, 4:07 PM
asomers requested changes to this revision.Jul 19 2018, 4:19 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
aniketp updated this revision to Diff 45552.Jul 19 2018, 5:42 PM
  • Fix 'pointer from integer without a cast' build failure on sparc64
Harbormaster completed remote builds in B18174: Diff 45552.Jul 19 2018, 5:42 PM
asomers accepted this revision.Jul 19 2018, 5:56 PM
This revision is now accepted and ready to land.Jul 19 2018, 5:56 PM
Closed by commit rS336875: audit(4): add tests for sysctl(3) and sysarch(2) (authored by asomers). · Explain WhyJul 29 2018, 8:35 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: imp. · View Herald TranscriptJul 29 2018, 8:35 PM

Revision Contents
Changeset List

PathSize
tests/
sys/
audit/
3 lines
232 lines

Diff 45552

View Options

tests/sys/audit/Makefile

Loading...
View Options

tests/sys/audit/miscellaneous.c

Loading...