Page MenuHomeFreeBSD
Differential D16116

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

Authored by aniketp on Jul 3 2018, 7:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 6, 8:45 AM
Unknown Object (File)
Thu, Apr 3, 10:37 PM
Unknown Object (File)
Sun, Mar 30, 8:19 AM
Unknown Object (File)
Mar 6 2025, 6:17 AM
Unknown Object (File)
Mar 5 2025, 5:15 AM
Unknown Object (File)
Mar 1 2025, 6:21 PM
Unknown Object (File)
Mar 1 2025, 1:15 PM
Unknown Object (File)
Feb 6 2025, 8:09 PM
View All Files
Subscribers
imp

Details

Reviewers
asomers
rwatson
gnn
Commits
rS339095: MFC r336875:
rS336875: audit(4): add tests for sysctl(3) and sysarch(2)
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

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 18035
Build 17785: arc lint + arc unit

Event Timeline

aniketp created this revision.Jul 3 2018, 7:15 PM
Harbormaster completed remote builds in B17822: Diff 44824.Jul 3 2018, 7:15 PM
aniketp updated this revision to Diff 44826.Jul 3 2018, 7:24 PM
  • Updates in Copyright
Harbormaster completed remote builds in B17823: Diff 44826.Jul 3 2018, 7:24 PM
asomers added a comment.Jul 3 2018, 7:29 PM

How about audit(2)?

aniketp updated this revision to Diff 44856.Jul 4 2018, 3:09 PM
  • Add test for audit(2) in failure case
Harbormaster completed remote builds in B17839: Diff 44856.Jul 4 2018, 3:09 PM
asomers added a comment.Jul 4 2018, 6:33 PM

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

asomers added inline comments.Jul 10 2018, 2:59 PM
tests/sys/audit/miscellaneous.c
261

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.

aniketp updated this revision to Diff 45280.Jul 14 2018, 12:08 PM

Create a single success mode test case for sysarch(2) with sysnum being dependent on the system architecture

Harbormaster completed remote builds in B18030: Diff 45280.Jul 14 2018, 12:08 PM
asomers accepted this revision.Jul 14 2018, 2:58 PM
asomers added inline comments.
tests/sys/audit/miscellaneous.c
100

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

This revision is now accepted and ready to land.Jul 14 2018, 2:58 PM
aniketp added inline comments.Jul 14 2018, 3:02 PM
tests/sys/audit/miscellaneous.c
100

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.

asomers added inline comments.Jul 14 2018, 3:40 PM
tests/sys/audit/miscellaneous.c
100

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

aniketp updated this revision to Diff 45286.Jul 14 2018, 4:34 PM

Include Sparc64 in the possible architectures for sysarch(2)

This revision now requires review to proceed.Jul 14 2018, 4:34 PM
Harbormaster completed remote builds in B18033: Diff 45286.Jul 14 2018, 4:34 PM
aniketp updated this revision to Diff 45287.Jul 14 2018, 4:39 PM

Add the missed \#endif statement

Harbormaster completed remote builds in B18034: Diff 45287.Jul 14 2018, 4:39 PM
aniketp updated this revision to Diff 45288.Jul 14 2018, 4:46 PM
  • Add the missed sysnum for sparc64 tests
Harbormaster completed remote builds in B18035: Diff 45288.Jul 14 2018, 4:46 PM
asomers requested changes to this revision.Jul 15 2018, 8:57 PM
asomers added inline comments.
tests/sys/audit/miscellaneous.c
123

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
aniketp updated this revision to Diff 45396.Jul 16 2018, 10:37 PM
  • Use respective args instead of a common void *sysarg
Harbormaster completed remote builds in B18088: Diff 45396.Jul 16 2018, 10:37 PM
asomers requested changes to this revision.Jul 16 2018, 11:10 PM
asomers added inline comments.
tests/sys/audit/miscellaneous.c
94

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

98

This should be a register_t, not a void*.

104

Did you mean to change the value of sysnum?

134

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
aniketp marked 9 inline comments as done.Jul 17 2018, 11:00 AM
aniketp updated this revision to Diff 45407.Jul 17 2018, 11:00 AM
  • Changes as suggested in the previous review
Harbormaster completed remote builds in B18094: Diff 45407.Jul 17 2018, 11:00 AM
asomers accepted this revision.Jul 17 2018, 2:06 PM
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
aniketp added inline comments.Jul 17 2018, 3:00 PM
tests/sys/audit/miscellaneous.c
103

@asomers, I have the semicolon here. Why would the build fail otherwise?

aniketp updated this revision to Diff 45424.EditedJul 17 2018, 7:24 PM
  • 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.

Harbormaster completed remote builds in B18102: Diff 45424.Jul 17 2018, 7:24 PM
asomers added a comment.Jul 17 2018, 8:14 PM
In D16116#346010, @aniketp wrote:
  • 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.

aniketp updated this revision to Diff 45545.Jul 19 2018, 4:07 PM
  • Fix build for ARM and Sparc64 architectures
Harbormaster completed remote builds in B18169: Diff 45545.Jul 19 2018, 4:07 PM
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
aniketp updated this revision to Diff 45552.Jul 19 2018, 5:42 PM
  • Fix 'pointer from integer without a cast' build failure on sparc64
Harbormaster completed remote builds in B18174: Diff 45552.Jul 19 2018, 5:42 PM
asomers accepted this revision.Jul 19 2018, 5:56 PM
This revision is now accepted and ready to land.Jul 19 2018, 5:56 PM
Closed by commit rS336875: audit(4): add tests for sysctl(3) and sysarch(2) (authored by asomers). · Explain WhyJul 29 2018, 8:35 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: imp. · View Herald TranscriptJul 29 2018, 8:35 PM

Revision Contents
Changeset List

PathSize
tests/
sys/
audit/
3 lines
225 lines

Diff 45288

View Options

tests/sys/audit/Makefile

# $FreeBSD$ # $FreeBSD$
SSP_CFLAGS= SSP_CFLAGS=
NO_WMISSING_VARIABLE_DECLARATIONS= NO_WMISSING_VARIABLE_DECLARATIONS=
.include <src.opts.mk> .include <src.opts.mk>
.if !defined(BUILDING_TESTS) .if !defined(BUILDING_TESTS)
OBJS+= Scrt1.o crt1.o gcrt1.o OBJS+= Scrt1.o crt1.o gcrt1.o
OBJS+= crtbegin.o crtbeginS.o crtbeginT.o OBJS+= crtbegin.o crtbeginS.o crtbeginT.o
OBJS+= crtend.o crtendS.o OBJS+= crtend.o crtendS.o
OBJS+= crti.o crtn.o OBJS+= crti.o crtn.o
CRT1OBJS+= crtbrand.o ignore_init_note.o CRT1OBJS+= crtbrand.o feature_note.o ignore_init_note.o
ACFLAGS+= -DLOCORE ACFLAGS+= -DLOCORE
CFLAGS+= -fno-asynchronous-unwind-tables CFLAGS+= -fno-asynchronous-unwind-tables
CFLAGS+= -fno-omit-frame-pointer CFLAGS+= -fno-omit-frame-pointer
CFLAGS+= -I${.CURDIR:H}/common \ CFLAGS+= -I${.CURDIR:H}/common \
-I${SRCTOP}/lib/libc/include -I${SRCTOP}/lib/libc/include
CFLAGS_CRTS= -DSHARED ${PICFLAG} CFLAGS_CRTS= -DSHARED ${PICFLAG}
FILES= ${OBJS} FILES= ${OBJS}
FILESMODE= ${LIBMODE} FILESMODE= ${LIBMODE}
FILESOWN= ${LIBOWN} FILESOWN= ${LIBOWN}
FILESGRP= ${LIBGRP} FILESGRP= ${LIBGRP}
FILESDIR= ${LIBDIR} FILESDIR= ${LIBDIR}
# These FILES qualify as libraries for the purpose of LIBRARIES_ONLY. # These FILES qualify as libraries for the purpose of LIBRARIES_ONLY.
.undef LIBRARIES_ONLY .undef LIBRARIES_ONLY
CLEANFILES+= ${OBJS} ${CRT1OBJS} crt1_c.o gcrt1_c.o Scrt1_c.o CLEANFILES+= ${OBJS} ${CRT1OBJS} crt1_c.o gcrt1_c.o Scrt1_c.o
CLEANFILES+= crti_s.o
crt1.o: crt1_c.o ${CRT1OBJS} crt1.o: crt1_c.o ${CRT1OBJS}
${LD} ${_LDFLAGS} -o ${.TARGET} -r ${.ALLSRC:M*.o} ${LD} ${_LDFLAGS} -o ${.TARGET} -r ${.ALLSRC:M*.o}
.if ${MACHINE_ARCH} == "i386" .if ${MACHINE_ARCH} == "i386"
${OBJCOPY} --localize-symbol _start1 ${.TARGET} ${OBJCOPY} --localize-symbol _start1 ${.TARGET}
.endif .endif
gcrt1_c.o: crt1_c.c gcrt1_c.o: crt1_c.c
Show All 18 Lines
crtendS.o: crtend.c crtendS.o: crtend.c
crtbegin.o crtend.o crtbeginT.o: crtbegin.o crtend.o crtbeginT.o:
${CC} ${CFLAGS} -I${.CURDIR} -c -o ${.TARGET} ${.ALLSRC:N*.h:[1]} ${CC} ${CFLAGS} -I${.CURDIR} -c -o ${.TARGET} ${.ALLSRC:N*.h:[1]}
crtbeginS.o crtendS.o: crtbeginS.o crtendS.o:
${CC} ${CFLAGS} -I${.CURDIR} ${CFLAGS_CRTS} -c -o ${.TARGET} \ ${CC} ${CFLAGS} -I${.CURDIR} ${CFLAGS_CRTS} -c -o ${.TARGET} \
${.ALLSRC:N*.h:[1]} ${.ALLSRC:N*.h:[1]}
crti_s.o: crti.S
${CC} ${CFLAGS} ${ACFLAGS} -c ${.ALLSRC:M*.S} -o ${.TARGET}
crti.o: crti_s.o crtbrand.o
jhbAuthorUnsubmitted
Done
Inline Actions

Here we link crti_s.o (generated from per-arch crti.S) and crtbrand.o to build crti.o. This is then included in both shared libraries and executables.

jhb: Here we link crti_s.o (generated from per-arch crti.S) and crtbrand.o to build crti.o. This is…
${LD} ${_LDFLAGS} -o ${.TARGET} -r ${.ALLSRC:M*.o}
.endif .endif
.include "../Makefile.inc" .include "../Makefile.inc"
View Options

tests/sys/audit/miscellaneous.c

# $FreeBSD$ # $FreeBSD$
SSP_CFLAGS= SSP_CFLAGS=
NO_WMISSING_VARIABLE_DECLARATIONS= NO_WMISSING_VARIABLE_DECLARATIONS=
.include <src.opts.mk> .include <src.opts.mk>
.if !defined(BUILDING_TESTS) .if !defined(BUILDING_TESTS)
OBJS+= Scrt1.o crt1.o gcrt1.o OBJS+= Scrt1.o crt1.o gcrt1.o
OBJS+= crtbegin.o crtbeginS.o crtbeginT.o OBJS+= crtbegin.o crtbeginS.o crtbeginT.o
OBJS+= crtend.o crtendS.o OBJS+= crtend.o crtendS.o
OBJS+= crti.o crtn.o OBJS+= crti.o crtn.o
CRT1OBJS+= crtbrand.o ignore_init_note.o CRT1OBJS+= crtbrand.o feature_note.o ignore_init_note.o
ACFLAGS+= -DLOCORE ACFLAGS+= -DLOCORE
CFLAGS+= -fno-asynchronous-unwind-tables CFLAGS+= -fno-asynchronous-unwind-tables
CFLAGS+= -fno-omit-frame-pointer CFLAGS+= -fno-omit-frame-pointer
CFLAGS+= -I${.CURDIR:H}/common \ CFLAGS+= -I${.CURDIR:H}/common \
-I${SRCTOP}/lib/libc/include -I${SRCTOP}/lib/libc/include
CFLAGS_CRTS= -DSHARED ${PICFLAG} CFLAGS_CRTS= -DSHARED ${PICFLAG}
FILES= ${OBJS} FILES= ${OBJS}
FILESMODE= ${LIBMODE} FILESMODE= ${LIBMODE}
FILESOWN= ${LIBOWN} FILESOWN= ${LIBOWN}
FILESGRP= ${LIBGRP} FILESGRP= ${LIBGRP}
FILESDIR= ${LIBDIR} FILESDIR= ${LIBDIR}
# These FILES qualify as libraries for the purpose of LIBRARIES_ONLY. # These FILES qualify as libraries for the purpose of LIBRARIES_ONLY.
.undef LIBRARIES_ONLY .undef LIBRARIES_ONLY
CLEANFILES+= ${OBJS} ${CRT1OBJS} crt1_c.o gcrt1_c.o Scrt1_c.o CLEANFILES+= ${OBJS} ${CRT1OBJS} crt1_c.o gcrt1_c.o Scrt1_c.o
CLEANFILES+= crti_s.o
crt1.o: crt1_c.o ${CRT1OBJS} crt1.o: crt1_c.o ${CRT1OBJS}
${LD} ${_LDFLAGS} -o ${.TARGET} -r ${.ALLSRC:M*.o} ${LD} ${_LDFLAGS} -o ${.TARGET} -r ${.ALLSRC:M*.o}
.if ${MACHINE_ARCH} == "i386" .if ${MACHINE_ARCH} == "i386"
${OBJCOPY} --localize-symbol _start1 ${.TARGET} ${OBJCOPY} --localize-symbol _start1 ${.TARGET}
.endif .endif
gcrt1_c.o: crt1_c.c gcrt1_c.o: crt1_c.c
Show All 18 Lines
crtendS.o: crtend.c crtendS.o: crtend.c
crtbegin.o crtend.o crtbeginT.o: crtbegin.o crtend.o crtbeginT.o:
${CC} ${CFLAGS} -I${.CURDIR} -c -o ${.TARGET} ${.ALLSRC:N*.h:[1]} ${CC} ${CFLAGS} -I${.CURDIR} -c -o ${.TARGET} ${.ALLSRC:N*.h:[1]}
crtbeginS.o crtendS.o: crtbeginS.o crtendS.o:
${CC} ${CFLAGS} -I${.CURDIR} ${CFLAGS_CRTS} -c -o ${.TARGET} \ ${CC} ${CFLAGS} -I${.CURDIR} ${CFLAGS_CRTS} -c -o ${.TARGET} \
${.ALLSRC:N*.h:[1]} ${.ALLSRC:N*.h:[1]}
crti_s.o: crti.S
${CC} ${CFLAGS} ${ACFLAGS} -c ${.ALLSRC:M*.S} -o ${.TARGET}
crti.o: crti_s.o crtbrand.o
jhbAuthorUnsubmitted
Done
Inline Actions

Here we link crti_s.o (generated from per-arch crti.S) and crtbrand.o to build crti.o. This is then included in both shared libraries and executables.

jhb: Here we link crti_s.o (generated from per-arch crti.S) and crtbrand.o to build crti.o. This is…
${LD} ${_LDFLAGS} -o ${.TARGET} -r ${.ALLSRC:M*.o}
.endif .endif
.include "../Makefile.inc" .include "../Makefile.inc"