Page MenuHomeFreeBSD

Correct the bit-value of ARG_TERMID_ADDR in sys/security/audit/audit-private.h
ClosedPublic

Authored by aniketp on Jun 8 2018, 3:50 AM.

Details

Summary

The definition of "ARG_TERMID_ADDR" macro assigns it a value of 0x0000000000400000ULL
which is already assigned to "ARG_SADDRUNIX" a line above.

Current state:

#define	ARG_SADDRUNIX		0x0000000000400000ULL
#define	ARG_TERMID_ADDR		0x0000000000400000ULL

Here, ARG_TERMID_ADDR should be 0x0000000000800000ULL

This change was accidentally introduced in rS168688.

Bug Report: 228820

Test Plan

Not needed

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 8 2018, 3:50 AM
aniketp updated this revision to Diff 43444.Jun 8 2018, 3:56 AM

Typo correction in audit.c: covert -> convert

The two comment typo fixes should not be part of this review, because they're unrelated. For minor things like comment typos it's fine to include them in another change to the same file, but I wouldn't throw them into a change in another file. Also, I disagree with the assertion that no testing is needed. I think this change deserves a regression test for all related audit events. That would be kill, pdkill, thr_kill, thr_kill2, ptrace, linux_tkill, and auditinfo_addr. But you haven't written tests for any of those yet. Let's hold off merging this DR until you've tested a few of those other syscalls.

aniketp updated this revision to Diff 43468.Jun 8 2018, 6:34 PM

Remove the typo corrections from this review.

Also, about testing the events mentioned above, I have already tested ptrace here:
ptrace_success
I'll work on creating tests for kill and killpg. And none of the other events mentioned are auditable.
e.g pdkill, linux_tkill auditaddr_info, thr_kill, thr_kill2 could not be audited as expected since they
are not mentioned in the ar->ar_event in sys/security/audit/audit_bsm.c

@asomers can this be landed? I've confirmed the successful regression test for ptrace(2) after changing the bit value and rebuilding the kernel. (Rest of the syscalls you mentioned are not auditable in FreeBSD)

asomers accepted this revision.Jun 13 2018, 2:53 PM
This revision is now accepted and ready to land.Jun 13 2018, 2:53 PM
This revision was automatically updated to reflect the committed changes.