Page MenuHomeFreeBSD

Add tests for miscellaneous administrative system calls
ClosedPublic

Authored by aniketp on Jun 19 2018, 6:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 20, 4:05 PM
Unknown Object (File)
Dec 20 2023, 7:12 AM
Unknown Object (File)
Dec 10 2023, 7:16 PM
Unknown Object (File)
Sep 18 2023, 4:52 AM
Unknown Object (File)
Sep 18 2023, 4:52 AM
Unknown Object (File)
Sep 18 2023, 4:51 AM
Unknown Object (File)
Sep 18 2023, 4:51 AM
Unknown Object (File)
Sep 18 2023, 4:50 AM
Subscribers

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

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17527
Build 17350: arc lint + arc unit

Event Timeline

asomers requested changes to this revision.Jun 19 2018, 4:42 PM
asomers added inline comments.
tests/sys/audit/administrative.c
172

timebuff is used uninitialized.

270

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

743

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

772

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
tests/sys/audit/administrative.c
172

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

270

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

743
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.
tests/sys/audit/administrative.c
172

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

270

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.

715

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

743

Does that even happen for Q_SYNC?

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

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

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

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

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

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

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

tests/sys/audit/administrative.c
772

Are you still working on mounting a filesystem?

tests/sys/audit/administrative.c
772

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

  • 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 set the repository for this revision to rS FreeBSD src repository - subversion.
  • Add tests for clock_settime(2)
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
  • 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
327

evclass is used uninitialized here.

This revision now requires changes to proceed.Jun 27 2018, 4:44 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
327

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
  • Initialize ec_class to 0
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.