Page MenuHomeFreeBSD

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

Authored by pawel.biernacki-gmail.com 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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

trasz added a subscriber: trasz.Nov 8 2017, 1:59 PM
trasz added inline comments.
usr.sbin/bhyve/uart_emul.c
693 ↗(On Diff #34905)

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

usr.sbin/bhyve/uart_emul.c
693 ↗(On Diff #34905)

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 ↗(On Diff #34905)

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!