Page MenuHomeFreeBSD

Remove references to pdwait4(2) and `CAP_PDWAIT` from rights(4)
ClosedPublic

Authored by ngie on Jan 25 2019, 12:08 AM.

Details

Summary

@cem removed references to pdwait4(2) (a nonexistent syscall) in
r320058.

This change removes references to pdwait4(2) and CAP_PDWAIT in
rights(4) to not mislead the user into thinking that pdwait4(2)/CAP_PDWAIT is
actually implemented in the stock FreeBSD kernel.

The goal of this functionality was to simplify monitoring/manipulating
processes started with pdfork, et al. The syscall was never completed
though--just discussed on the capsicum mailing list back in 2015:
https://lists.cam.ac.uk/pipermail/cl-capsicum-discuss/2015-May/msg00012.html
. That being said, there are members of the project (@rwatson, etc) who
have longterm goals to implement this syscall to better secure pdfork(2)
calls.

PR: 235871

Test Plan

make universe passed.

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

ngie created this revision.Jan 25 2019, 12:08 AM
ngie edited the summary of this revision. (Show Details)Jan 25 2019, 12:28 AM
ngie added a subscriber: drysdale_google.com.
cem removed a reviewer: cem.Jan 25 2019, 12:29 AM
cem removed a subscriber: cem.
ngie edited the summary of this revision. (Show Details)Jan 25 2019, 12:30 AM
ngie edited the test plan for this revision. (Show Details)
ngie added a subscriber: cem.

I think rS339356 is the wrong rev

ngie edited the summary of this revision. (Show Details)Feb 5 2019, 8:18 AM

I think rS339356 is the wrong rev

You're right -- it should have been rS320058.

ngie edited the test plan for this revision. (Show Details)Feb 5 2019, 6:14 PM

Not documenting it (as in rS320058) is reasonable, but I'm less sure about removing the bit definition unless we're certain we never intend to implement it.

ngie edited the summary of this revision. (Show Details)Feb 6 2019, 9:25 PM
ngie updated this revision to Diff 53630.Feb 6 2019, 9:50 PM

Chase proposed change by updating CAP_ALL1/CAP_UNUSED1_22 with the removed cap bit.

ngie added a comment.Feb 6 2019, 10:27 PM

Not documenting it (as in rS320058) is reasonable, but I'm less sure about removing the bit definition unless we're certain we never intend to implement it.

I talked with kib@ a bit about this and [most] of what pdwait4 aimed to do is doable with kqueue/kevent. The only thing that pdwait4 could potentially do that the existing implementation doesn't do is deal with getting the exit status from dead PIDs.

Just to be safe and to "do the right thing" I updated the ALL/UNUSED bit masks to be correct.

jtl added reviewers: pjd, jonathan.EditedFeb 12 2019, 3:00 AM

@jonathan , @pjd : Does this change look okay to you? You both committed some of the bits being reverted (although, its been 7-8 years since then).

IMO it's fair to remove from the man page now, but I would like feedback from capsicum for the source changes.

rwatson requested changes to this revision.Feb 18 2019, 12:59 PM

Remove man-page references seems reasonable, since the man page clearly isn't there. But I'd leave the constants as-is to hold the constants / strings and avoid surprising issues with backward compatibility. I think it still does make sense to implement pdwait4(2), as collecting status information will be important to future capsicum-enabled shells, with helper applications, etc. Adding a comment in the source and in the permissions description in the man page about it being a placeholder would be reasonable.

This revision now requires changes to proceed.Feb 18 2019, 12:59 PM
ngie updated this revision to Diff 54104.Feb 20 2019, 1:50 AM

Remove CAP_PDWAIT from rights(4) and add a comment to sys/capsicum.h instead of removing the constant.

This restores other changes (lib/libprocstat, tools/regression) to their versions on ^/head

ngie retitled this revision from Remove references to pdwait4(2) and `CAP_PDWAIT` to Remove references to pdwait4(2) and `CAP_PDWAIT` from rights(4).Feb 20 2019, 1:58 AM
ngie edited the summary of this revision. (Show Details)
ngie updated this revision to Diff 54105.Feb 20 2019, 2:01 AM
ngie edited the summary of this revision. (Show Details)

Update the commit message and reference the bug in sys/capsicum.h

One further suggestion, I'm afraid: BUGS entries in capsicum(4) and rights(4) suggesting pdwait4(2) and CAP_PDWAIT?

emaste accepted this revision.Feb 25 2019, 3:30 PM
ngie added a comment.Feb 28 2019, 6:07 PM

One further suggestion, I'm afraid: BUGS entries in capsicum(4) and rights(4) suggesting pdwait4(2) and CAP_PDWAIT?

I can do that in a separate commit.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 28 2019, 6:12 PM
This revision was automatically updated to reflect the committed changes.