Page MenuHomeFreeBSD

bhyve: don't try to capsicumise after failed open
ClosedPublic

Authored by kaktus on Nov 7 2017, 11:22 PM.

Details

Summary

When user specifies -l option and the target file can't be opened (ie. nmdm module isn't loaded and /dev/nmdm* is specified) bhyve tries to apply capabilities to not open file. Enclose that part in an if statement and only run it on correctly opened descriptor and in case of error provide meaningful message about the problem.

Test Plan

After applying this patch, specifying non existing file (or not loading nmdm) to -l option should effect in

Unable to initialize backend '/dev/asdf' for LPC device COM1

instead of

Unable to apply rights for sandbox

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

kaktus created this revision.Nov 7 2017, 11:22 PM
kaktus edited the summary of this revision. (Show Details)Nov 7 2017, 11:56 PM
trasz added a subscriber: trasz.Nov 8 2017, 1:59 PM
trasz added inline comments.
usr.sbin/bhyve/uart_emul.c
693

Use err(), perhaps, to not hide the error string?

kaktus added inline comments.Nov 9 2017, 2:33 PM
usr.sbin/bhyve/uart_emul.c
693

Although I'm not saying no to this, it'll not help much in this case.
If @grehan will be interested I can prepare separate patch and replace all occurrences of errx with err in the capsicum cases.

grehan accepted this revision.Nov 9 2017, 2:40 PM

Thanks for this patch.

The err vs errx can be a separate discussion.

This revision is now accepted and ready to land.Nov 9 2017, 2:40 PM
emaste accepted this revision.Nov 9 2017, 2:41 PM
emaste added inline comments.
usr.sbin/bhyve/uart_emul.c
693

I agree that such a change ought to be a separate patch, but also agree it is probably not valuable to have the error.

robak added a comment.Nov 9 2017, 2:52 PM

@grehan @emaste thanks for review guys, will commit it asap!