Page MenuHomeFreeBSD

Capsicumize fsck_msdosfs
Needs ReviewPublic

Authored by shubh on Jul 31 2020, 1:21 PM.


  • Added capsicum support to sandbox fsck_msdosfs.
  • Added HAVE_CAPSICUM, so that the code can be used on different platforms
  • Added cap_fileargs support
Test Plan

ktrace fsck_msdosfs <device_msdosfs_1> <device_msdosfs_2>
kdump | grep cap

truss fsck_msdosfs <device_msdosfs> <device_msdosfs_2>

Diff Detail

Lint Skipped
Unit Tests Skipped

Event Timeline

shubh requested review of this revision.Jul 31 2020, 1:21 PM

Don't we need a makefile change as well, to set -DWITH_CASPER?


Style, missing a space after #include.

We also don't need the blank line between this and the rest of the includes.


Judging from this #ifdef I guess we'll want an explicit #ifdef HAVE_CASPER rather than relying on capsicum_helpers?

Looks good to me as long as the capscium specific code were wrapped with #ifdef's as this is shared with other platforms.

shubh edited the test plan for this revision. (Show Details)
  • Added #ifdef HAVE_CAPSICUM

This open() will always fail now.

  • Minor fix to let all cases of open() calls work

Look at how this function is called in main(). If multiple filesystems are passed on the command line, with this patch we will fail after the first. You would need to use cap_fileargs to open them.

shubh edited the summary of this revision. (Show Details)
shubh edited the test plan for this revision. (Show Details)
  • Added cap_fileargs for multiple filesystems as arguments.
  • 2 instances of cap_fileargs have been used to imitate the open() calls for different flag cases
  • Wrapped the sandboxing logic under HAVE_CAPSICUM flag

We don't need this else part. The fileargs_open will be converted to normal open if capser is unavilable.

35 ↗(On Diff #75237)

Why we are using HAVE_CAPSICUM.

39 ↗(On Diff #75237)

Wrong sort. We went over this in the other review.

53 ↗(On Diff #75237)

This is not a good practice to have everything as a global variables. Maybe we can pass this as a arguments to the function?

126 ↗(On Diff #75237)

fileargs_init creates an casper instance, it's not a big deal but we may use fileargs_cinit and create casper only once.

132 ↗(On Diff #75237)

I'm not sure I'm getting this cap_right_init part. The ident is broken.

142 ↗(On Diff #75237)


35 ↗(On Diff #75237)

HAVE_CAPSICUM is to address @delphij's note that this code is used on other platforms (Android, specifically) where Capsicum is not available.

  • Code for opening the filesystem has been shifted to main.c so that different cases for HAVE_CAPSICUM could use a similar call to checkfilesys()
  • Added caph_enter_caspe() instead of caph_enter()
  • Other minor fixes
147 ↗(On Diff #75918)