Page MenuHomeFreeBSD

Add tests for miscellaneous administrative system calls
ClosedPublic

Authored by aniketp on Jun 19 2018, 6:07 AM.

Details

Summary

This revision introduces atf-c(3) tests for many unrelated administrative system calls.
Note: Most the syscalls are too critical to be tested in success mode.

List of Syscalls:

  • ntp_adjtime(2)
  • auditctl(2)
  • acct(2)
  • auditon(2)
  • clock_settime(2)

// Only in failure mode

  • reboot(2)
  • quotactl(2)
  • mount(2)
  • nmount(2)
  • swapon(2)
  • swapoff(2)
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

aniketp created this revision.Jun 19 2018, 6:07 AM
asomers requested changes to this revision.Jun 19 2018, 4:42 PM
asomers added inline comments.
tests/sys/audit/administrative.c
171 ↗(On Diff #44061)

timebuff is used uninitialized.

267 ↗(On Diff #44061)

What happens to the system's audit trail after this command? Does it keep recording the stuff in /var/audit?

656 ↗(On Diff #44061)

Can't you test quotactl successfully using the non-destructive Q_GETQUOTASIZE, Q_GETQUOTA, or Q_SYNC commands?

685 ↗(On Diff #44061)

Actually, you can mount a filesystem. nullfs would work, for example. But be sure to unmount it during the cleanup!

This revision now requires changes to proceed.Jun 19 2018, 4:42 PM
aniketp added inline comments.Jun 20 2018, 11:06 AM
tests/sys/audit/administrative.c
171 ↗(On Diff #44061)

Is it not supposed to be filled by ntp_adjtime(2). That is what man-page suggested.

267 ↗(On Diff #44061)

After I call auditctl(2) myself, auditing starts in my configured path rather than /var/audit.
But when the audit daemon is stopped and started. Process resumes in /var/audit

Actually:
When the auditd(8) is not already running, everything works fine after running the tests. But on the other side, auditing in /var/audit stops.

So basically, to make everything work normally, we'll have to stop and start auditd8) again. (I verified this behaviour using an independent program).

656 ↗(On Diff #44061)
int size;
if (quotactl("/", Q_GETQUOTASIZE, 0, &size) < 0)
        perror("quotactl");

outputs:
quotactl: Invalid argument

From the man-page, it looks like the error is because the filesystem quota is not already enabled on my system

MAN
The cmd argument or the command type is invalid.  In
Q_GETQUOTASIZE, Q_GETQUOTA, Q_SETQUOTA, and Q_SETUSE,
quotas are not currently enabled for this file system.
asomers added inline comments.Jun 20 2018, 3:20 PM
tests/sys/audit/administrative.c
171 ↗(On Diff #44061)

No, you're misreading it. That argument is provided from userland to the kernel.

267 ↗(On Diff #44061)

Tough call. Do we skip the test if auditd is already running? Or do we run it anyway, but make sure to restart auditd in the cleanup? I'm leaning toward the latter, even though it will cause a short disruption to the system-wide audit trail. My reasoning is that none of the ATF tests should be run in a production environment, so a brief audit-trail disruption should be ok.

628 ↗(On Diff #44061)

If we're going to temporarily disrupt the audit trail, then we may as well disrupt process accounting, too. You can check whether accounting is enabled with the kern.acct_configured sysctl. Then, during cleanup, either do "service accounting onestop" (if it wasn't enabled) or "service accounting restart" (if it was).

656 ↗(On Diff #44061)

Does that even happen for Q_SYNC?

aniketp updated this revision to Diff 44203.Jun 21 2018, 8:51 AM
aniketp marked 10 inline comments as done.
  • Add test for acct_success. Use sysctlbyname(3) to retrieve the accounting status
  • Minor changes in auditctl_success test case cleanup
asomers requested changes to this revision.Jun 21 2018, 2:47 PM
asomers added inline comments.
tests/sys/audit/administrative.c
346 ↗(On Diff #44203)

Why go to so much effort not to enable accounting? It would be simpler to make acctpath a constant. It will be deleted anyway at the end of the test.

357 ↗(On Diff #44203)

You can't do cleanup in the test body, because it won't run if the test fails. You need to move this to the cleanup function.

This revision now requires changes to proceed.Jun 21 2018, 2:47 PM
aniketp updated this revision to Diff 44237.Jun 21 2018, 4:01 PM

Update the test case for acct_success

asomers requested changes to this revision.Jun 21 2018, 7:21 PM
asomers added inline comments.
tests/sys/audit/administrative.c
335 ↗(On Diff #44237)

It's not useful to document that the file is conditional; the condition one line above makes that clear. If you're going to document it, it should be to highlight its role in the cleanup function.

356 ↗(On Diff #44237)

Pro tip: if you initialize filedesc2 to -1, then you can unconditionally close it and ignore the return value. However, in this case it would be better to move the close statement up to line 336, immediately after opening it.

This revision now requires changes to proceed.Jun 21 2018, 7:21 PM
aniketp updated this revision to Diff 44272.Jun 21 2018, 11:25 PM

Shift the location of close(filedesc2) to right after opening it

asomers added inline comments.Jun 22 2018, 2:57 AM
tests/sys/audit/administrative.c
685 ↗(On Diff #44061)

Are you still working on mounting a filesystem?

aniketp added inline comments.Jun 22 2018, 4:27 AM
tests/sys/audit/administrative.c
685 ↗(On Diff #44061)

I've been trying out numerous variations of commands to see if they execute successfully for mount(2) and nmount(2).

Whenever I get the "type" right, i.e either "nullfs" or "tmpfs", Any other set of command outputs Operation not supported.
For the wrong types, I expectedly get invalid argument

aniketp edited the summary of this revision. (Show Details)Jun 22 2018, 4:27 AM
aniketp updated this revision to Diff 44525.Jun 27 2018, 1:59 PM
  • Add tests for auditon(2), swapon(2) and swapoff(2)

Note: swapon(2) and swapoff(2) require block device, hence can't be tested
in success case since block devices are gone (from FreeBSD)

aniketp edited the summary of this revision. (Show Details)Jun 27 2018, 2:00 PM
aniketp set the repository for this revision to rS FreeBSD src repository.
aniketp updated this revision to Diff 44527.Jun 27 2018, 2:53 PM
  • Add tests for clock_settime(2)
aniketp edited the summary of this revision. (Show Details)Jun 27 2018, 3:04 PM
asomers requested changes to this revision.Jun 27 2018, 3:39 PM

If you intend to add successful test cases for mount, swapon, etc later, then you shouldn't indicate them as TODOs in the comments. Remove the text about how mount can't be tested in success mode.

This revision now requires changes to proceed.Jun 27 2018, 3:39 PM
aniketp updated this revision to Diff 44531.Jun 27 2018, 4:19 PM
  • Remove placeholder comment about not testing mount(2), nmount(2) in success case

These will be dealt later on

asomers requested changes to this revision.Jun 27 2018, 4:44 PM
asomers added inline comments.
tests/sys/audit/administrative.c
385 ↗(On Diff #44531)

evclass is used uninitialized here.

This revision now requires changes to proceed.Jun 27 2018, 4:44 PM
aniketp updated this revision to Diff 44541.Jun 27 2018, 5:11 PM

Initialize evclass to get the event-class mapping for auditon(2)

asomers requested changes to this revision.Jun 27 2018, 5:20 PM
asomers added inline comments.
tests/sys/audit/administrative.c
386 ↗(On Diff #44541)

You should initialize ec_class too, even though current kernel code doesn't use it. Otherwise, tools like Coverity will detect this as a use of uninitialized data. Anything will work. Either bzero, or an explicit value.

This revision now requires changes to proceed.Jun 27 2018, 5:20 PM
aniketp updated this revision to Diff 44545.Jun 27 2018, 5:28 PM
  • Initialize ec_class to 0
aniketp marked 6 inline comments as done.Jun 27 2018, 5:30 PM
asomers accepted this revision.Jun 29 2018, 4:40 AM
This revision is now accepted and ready to land.Jun 29 2018, 4:40 AM
This revision was automatically updated to reflect the committed changes.