capsicum bspatch(1)

[emaste]

Summary

Refactor to first open all fds and FILE *s, and move processing after cap_enter.

[oshogbo]

I'm not sure if you spoke with AllanJude but he proposed patch https://reviews.freebsd.org/D7358 for bspatch .
Capsicumizing looks good for me.

There was a general request in that patch: "People downstream bspatch in other projects. Can we ifdef this based on something?", in tcpdump we ifdef it based on FreeBSD.

usr.bin/bsdiff/bspatch/bspatch.c

30

sys should be in separate block, right?

#include <sys/capsicum.h>

#include <...>

133

I'm not sure about style of this file.
If we want be correct with style(9) then I think it should be:
open(argv[1], O_RDONLY | O_BINARY, 0);

The same with:
open(argv[2], O_CREAT | O_TRUNC | O_WRONLY | O_BINARY, 0666);

136

delphij@ pointed out in original patch, that this file should be removed if any other error accrues.

[emaste]

Oh, I wasn't aware of D7358 - I'll discuss with @allanjude.

usr.bin/bsdiff/bspatch/bspatch.c

133

Yes, the style in here is already a mess, but you're right that these should be fixed.

In fact I think we should perhaps first style(9) this file (or at least the parts we're going to be changing -- file opening, seeking etc.), to make the subsequent work for capsicum more clear.

136

Yes, indeed.

[emaste]

Yes, we can #ifdef it easily. For elftoolchain I proposed this:

https://github.com/emaste/elftoolchain/commit/ce3c99c295001084aaaa7da29770797f3b268fa9#diff-0b00df06b17cf879b89570db09feec5eR48

not sure if we should bother with the version checks in-tree.

[allanjude]

In D7603#158129, @emaste wrote:

Yes, we can #ifdef it easily. For elftoolchain I proposed this:

https://github.com/emaste/elftoolchain/commit/ce3c99c295001084aaaa7da29770797f3b268fa9#diff-0b00df06b17cf879b89570db09feec5eR48

not sure if we should bother with the version checks in-tree.

I like your ifdef style there.

I also like delphij's idea of, in the case of there not being capsicum, chroot to /var/empty, but this requires root. I have a modified version of my patch that tries to address this, but it started to get ugly. I can post it later today

[emaste]

• fix typo spotted by @allanjude
• remove unnecessary brackets

[emaste]

oops, I updated the wrong review :(

[allanjude]

I have updated my diff here:
https://reviews.freebsd.org/D7358

[emaste]

Already in progress by @allanjude in D7358