Page MenuHomeFreeBSD
Differential D15897

System V standardized IPC syscalls concerned with manipulating semaphore sets
ClosedPublic
Actions

Authored by aniketp on Jun 19 2018, 3:57 AM.
Tags
None
Referenced Files
F110274125: D15897.id44058.diff
Sun, Feb 16, 2:07 AM
F110261885: D15897.id44145.diff
Sat, Feb 15, 9:57 PM
Unknown Object (File)
Wed, Jan 29, 6:36 AM
Unknown Object (File)
Sat, Jan 25, 7:40 PM
Unknown Object (File)
Sat, Jan 25, 7:39 PM
Unknown Object (File)
Fri, Jan 24, 5:41 PM
Unknown Object (File)
Sat, Jan 18, 5:53 PM
Unknown Object (File)
Sat, Jan 18, 5:18 PM
View All Files
Subscribers
imp

Details

Reviewers
asomers
rwatson
gnn
Commits
rS335528: audit(4): add tests for Sys V semaphore operations
Summary

This revision introduces test cases for System V standardized system calls concerned
with creating and manipulating semaphore sets. These system calls are categorized
within inter-process-communication audit class (as expected).

System calls in question are:

  • semget(2)
  • semop(2)
  • semctl(2)

Note:
semctl(2) takes as argument a cmd which is a separate audit event in itself.
List of semctl(2) audit events:

  • AUE_SEMCTL [Illegal Command]
  • AUE_SEMCTL_GETALL
  • AUE_SEMCTL_GETNCNT
  • AUE_SEMCTL_GETZCNT
  • AUE_SEMCTL_GETPID
  • AUE_SEMCTL_GETVAL
  • AUE_SEMCTL_SETALL
  • AUE_SEMCTL_SETVAL
  • AUE_SEMCTL_STAT
  • AUE_SEMCTL_RMID
  • AUE_SEMCTL_SET

For full details, please see: sys/security/audit/audit_bsm_klib.c#L271

Each of them have been treated as separate audit events. (And hence, separate tests)

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 created this revision.Jun 19 2018, 3:57 AM
Harbormaster completed remote builds in B17434: Diff 44058.Jun 19 2018, 3:57 AM
asomers requested changes to this revision.Jun 19 2018, 4:14 PM
asomers added inline comments.
tests/sys/audit/inter-process.c
49 ↗(On Diff #44058)

This is defined in sem.h. Don't redefine it here. Just #define _WANT_SEMUN before including sem.h.

53 ↗(On Diff #44058)

IMHO, "arg" is too vague a name for a global variable.

780 ↗(On Diff #44058)

There's actually nothing in the spec that says this should fail. However, the man page does say that semget should fail in either of the two following situations:

  • key is IPC_PRIVATE and nsems is <= 0
  • key is IPC_PRIVATE, and flags does not include IPC_CREAT.
810 ↗(On Diff #44058)

sizeof(sop) would be safer.

858 ↗(On Diff #44058)

arg->array is used uninitialized here. But GETVAL doesn't need the fourth argument, anyway.

961 ↗(On Diff #44058)

GETPID does not require the fourth argument.

1012 ↗(On Diff #44058)

GETNCNT doesn't require arg, either.

1167 ↗(On Diff #44058)

semvals is used uninitialized.

1270 ↗(On Diff #44058)

It makes no sense to initialize two fields of a union, because initializing the second one will wipe out the first.

This revision now requires changes to proceed.Jun 19 2018, 4:14 PM
aniketp updated this revision to Diff 44145.Jun 20 2018, 11:51 AM
aniketp marked 7 inline comments as done.

Replace arg with semarg; And some minor changes

Harbormaster completed remote builds in B17478: Diff 44145.Jun 20 2018, 11:51 AM
aniketp added inline comments.Jun 20 2018, 11:55 AM
tests/sys/audit/inter-process.c
1167 ↗(On Diff #44058)

semvals would be filled up by the SETALL command. Man-page doesn't say we need to explicitely initialize it.

asomers requested changes to this revision.Jun 20 2018, 2:59 PM
asomers added inline comments.
tests/sys/audit/inter-process.c
858 ↗(On Diff #44058)

Why don't you just remove the fourth argument and invoke it as semctl(semid, 0, GETVAL)?

1167 ↗(On Diff #44058)

I don't see that at all. Are you thinking about GETALL?

This revision now requires changes to proceed.Jun 20 2018, 2:59 PM
aniketp updated this revision to Diff 44151.Jun 20 2018, 3:51 PM
  • Remove semarg from GETVAL, GETPID, GETZCNT, GETNCNT
  • Initialize semvals in SETALL by first calling GETALL
Harbormaster completed remote builds in B17481: Diff 44151.Jun 20 2018, 3:51 PM
aniketp added inline comments.Jun 20 2018, 3:51 PM
tests/sys/audit/inter-process.c
858 ↗(On Diff #44058)

Oops! I had removed them from all GET* commands. Must have crept back it when I git stashed the changes.
Will remove them again.

asomers added a comment.Jun 20 2018, 4:15 PM

Don't forget the _WANT_SEMUN change.

tests/sys/audit/inter-process.c
1391 ↗(On Diff #44151)

Technically you're still using semarg uninitialized here. I recommend simply removing it.

aniketp updated this revision to Diff 44273.Jun 21 2018, 11:59 PM

Define _WANT_SEMUN instead of declaring union semun in the test program

Harbormaster completed remote builds in B17528: Diff 44273.Jun 21 2018, 11:59 PM
asomers accepted this revision.Jun 22 2018, 12:54 AM
This revision is now accepted and ready to land.Jun 22 2018, 12:54 AM
Closed by commit rS335528: audit(4): add tests for Sys V semaphore operations (authored by asomers). · Explain WhyJun 22 2018, 2:56 AM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: imp. · View Herald TranscriptJun 22 2018, 2:56 AM

Revision Contents
Changeset List

PathSize
head/
tests/
sys/
audit/
698 lines

Diff 44285

View Options

head/tests/sys/audit/inter-process.c

Loading...