Page MenuHomeFreeBSD

Capsicumize fsck_msdosfs
Needs ReviewPublic

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

Details

Summary
  • 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
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

shubh created this revision.Fri, Jul 31, 1:21 PM
shubh requested review of this revision.Fri, Jul 31, 1:21 PM
markj added a comment.Fri, Jul 31, 1:42 PM

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

sbin/fsck_msdosfs/check.c
36

Style, missing a space after #include.

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

36

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 updated this revision to Diff 75227.Fri, Jul 31, 6:58 PM
shubh edited the test plan for this revision. (Show Details)
  • Added #ifdef HAVE_CAPSICUM
markj added inline comments.Fri, Jul 31, 7:06 PM
sbin/fsck_msdosfs/check.c
71–72

This open() will always fail now.

shubh updated this revision to Diff 75230.Fri, Jul 31, 7:15 PM
  • Minor fix to let all cases of open() calls work
markj added inline comments.Fri, Jul 31, 8:02 PM
sbin/fsck_msdosfs/check.c
49

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 updated this revision to Diff 75237.Sat, Aug 1, 5:10 AM
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
oshogbo added inline comments.Mon, Aug 3, 9:23 AM
sbin/fsck_msdosfs/check.c
64

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

sbin/fsck_msdosfs/ext.h
35

Why we are using HAVE_CAPSICUM.

39

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

53

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

sbin/fsck_msdosfs/main.c
126

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

132

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

142

caph_enter_casper?

markj added inline comments.Tue, Aug 4, 2:13 PM
sbin/fsck_msdosfs/ext.h
35

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