Page MenuHomeFreeBSD

Introduce test-program for process-control group of system calls
ClosedPublic

Authored by aniketp on Jun 22 2018, 3:45 PM.
Tags
None
Referenced Files
F106194242: D15966.id44306.diff
Thu, Dec 26, 11:01 PM
Unknown Object (File)
Thu, Dec 19, 5:01 PM
Unknown Object (File)
Sat, Dec 14, 5:26 PM
Unknown Object (File)
Fri, Dec 13, 8:37 AM
Unknown Object (File)
Mon, Dec 9, 1:26 AM
Unknown Object (File)
Mon, Dec 2, 12:56 PM
Unknown Object (File)
Sat, Nov 30, 2:20 AM
Unknown Object (File)
Nov 26 2024, 9:34 AM
Subscribers
None

Details

Summary

This revision introduces a new (and final) test-program for syscalls categorized within
process-control audit class. A lot of these syscalls don't have tests for either
success or failure case due to various reasons

List of System calls

  1. Both cases
  • rfork(2)
  • chdir(2)
  • fchdir(2)
  • chroot(2)
  • getresuid(2)
  • getresgid(2)
  • setpriority(2)
  • setgroups(2)
  • setpgrp(2)
  • setrlimit(2)
  • setlogin(2)
  • mlock(2)
  • munlock(2)
  • minherit(2)
  • rtprio(2)
  • profil(2)
  • ktrace(2)
  • ptrace(2)
  1. Success case
  • fork(2)
  • umask(2)
  • setuid(2)
  • setgid(2)
  • seteuid(2)
  • setegid(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 - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

aniketp set the repository for this revision to rS FreeBSD src repository - subversion.

Add test for successful invocation of setlogin(2)

asomers requested changes to this revision.Jun 22 2018, 5:39 PM
asomers added inline comments.
tests/sys/audit/process-control.c
72 ↗(On Diff #44309)

You could move check_audit up a line. In fact, it isn't really necessary to call wait at all.

75 ↗(On Diff #44309)

FYI, calling exit(3) after a fork is surprisingly risky. That's because it tries to do various cleanup activities. Usually those cleanup activities will work. But if the parent process was multithreaded, then the child will be single-threaded, and its only thread will be a copy of the parent thread that forked. That means that mutexes could end up leaked and other nastiness. That's why it's a common pattern to instead call _exit(2) after forking.

It doesn't matter here, because your program is single-threaded and doesn't have any mutexes or other stuff that could interfere with the child. Just keep it in mind for the future.

158 ↗(On Diff #44309)

Kyua is smart enough to chdir back to the test directory for cleanup. So you could simplify this test by simply chdir()ing to "/".

660 ↗(On Diff #44309)

It's fine to set priority to a hardcoded value like 0. This process will exit pretty soon anyway.

707 ↗(On Diff #44309)

gidset is used uninitialized.

740 ↗(On Diff #44309)

Don't forget to add this testcase.

1156 ↗(On Diff #44309)

Probably safer to use pid instead of hardcoding 0.

This revision now requires changes to proceed.Jun 22 2018, 5:39 PM
aniketp marked 5 inline comments as done.

Changes according to the inline comments

tests/sys/audit/process-control.c
699 ↗(On Diff #44334)

Safer to pass the full gidset to setgroups instead of only the first group.

Update ngroups for setgroups_success as 5 instead of 100

tests/sys/audit/process-control.c
699 ↗(On Diff #44334)

@asomers setgroups is peculiar in this regard.
If I pass in entire gidset array as the maximum limit it will never let the audit record complete itself.

The piece of code here:

	case AUE_SETGROUPS:
		if (ARG_IS_VALID(kar, ARG_GROUPSET)) {
			for(ctr = 0; ctr < ar->ar_arg_groups.gidset_size; ctr++)
			{
				tok = au_to_arg32(1, "setgroups",
				    ar->ar_arg_groups.gidset[ctr]);
				kau_write(rec, tok);
			}
		}
		break;

Loops till all gidset elements are inserted in the audit record. So for 100 such groups, I get an incomplete audit record and tests fail.
This doesn't happen with 1 or 5 as the limit.

With ngroups = 100;

Looking for 'setgroups.*1844.*ret.*success' in 'header,1868,11,setgroups(2),0,Sat Jun 23 19:15:30 2018, + 663 msecargument,1,0x0,setgroupsargument,1,0x0,setgroupsargument,1,0x5,setgroupsargument,1,0x0,setgroupsargument,1,0xe7913e01,setgroupsargument,1,0x8078a49c,setgroupsargument,1,0x635090,setgroupsargument,1,0x8,setgroupsargument,1,0xffffd480,setgroupsargument,1,0x7fff,setgroupsargument,1,0x77bf89,setgroupsargument,1,0x8,setgroupsargument,1,0x635090,setgroupsargument,1,0x8,setgroupsargument,1,0xe7913ef0,setgroupsargument,1,0x8078a49c,setgroupsargument,1,0xe7913ef0,setgroupsargument,1,0x8078a49c,setgroupsargument,1,0x1,setgroupsargument,1,0x0,setgroupsargument,1,0x635250,setgroupsargument,1,0x8,setgroupsargument,1,0xe7913ef0,setgroupsargument,1,0x8078a49c,setgroupsargument,1,0x1,setgroupsargument,1,0x0,setgroupsargument,1,0x64d010,setgroupsargument,1,0x8,setgroupsargument,1,0x635090,setgroupsargument,1,0x8,setgroupsargument,1,0xffffd3e0,setgroupsargument,1,0x7fff,setgroupsargument,1,0xffffd3a0,setgroupsargument,1,0x7fff,setgroupsargument,1,0x7d6869,setgro����'

process-control:setgroups_success  ->  failed: setgroups.*1844.*ret.*success not found in auditpipe within the time limit

Note: for the sake of the test case, I'm using ngroups as 5 instead of 100.

asomers added inline comments.
tests/sys/audit/process-control.c
699 ↗(On Diff #44334)

Looks like you're running up against the 1024 character limit imposed in utils.c:52. So, not a bug in audit(4). A 5-group limit is fine.

BTW, you might see a 16-group limit some places. That comes from NFSv2 and NFSv3, which set that limit in the RPCs.

This revision is now accepted and ready to land.Jun 23 2018, 7:57 PM

Add test for setpgrp_success

This revision now requires review to proceed.Jun 23 2018, 8:35 PM
aniketp set the repository for this revision to rS FreeBSD src repository - subversion.
aniketp removed a subscriber: imp.
This revision is now accepted and ready to land.Jun 25 2018, 12:42 AM
This revision was automatically updated to reflect the committed changes.