Page MenuHomeFreeBSD

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

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
Subscribers

Details

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

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

  • Add test for audit(2) in failure case

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

tests/sys/audit/miscellaneous.c
260 ↗(On Diff #44856)

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

asomers added inline comments.
tests/sys/audit/miscellaneous.c
99 ↗(On Diff #45280)

I'm curious. Why is sysarch unsafe on sparc?

This revision is now accepted and ready to land.Jul 14 2018, 2:58 PM
tests/sys/audit/miscellaneous.c
99 ↗(On Diff #45280)

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.

tests/sys/audit/miscellaneous.c
99 ↗(On Diff #45280)

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

Add the missed \#endif statement

  • Add the missed sysnum for sparc64 tests
asomers requested changes to this revision.Jul 15 2018, 8:57 PM
asomers added inline comments.
tests/sys/audit/miscellaneous.c
123 ↗(On Diff #45288)

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
asomers added inline comments.
tests/sys/audit/miscellaneous.c
93 ↗(On Diff #45396)

Technically this is an input/output argument, so you still need to initialize all fields or tools like Coverity will complain.

97 ↗(On Diff #45396)

This should be a register_t, not a void*.

103 ↗(On Diff #45396)

Did you mean to change the value of sysnum?

133 ↗(On Diff #45396)

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
  • Changes as suggested in the previous review
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
tests/sys/audit/miscellaneous.c
102 ↗(On Diff #45407)

@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.

  • 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
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
  • Fix 'pointer from integer without a cast' build failure on sparc64
This revision is now accepted and ready to land.Jul 19 2018, 5:56 PM
This revision was automatically updated to reflect the committed changes.