Page MenuHomeFreeBSD

ipfw: fix jail option

Authored by kevans on Aug 1 2019, 4:01 PM.



rS348215 changed jail_getid(3) to validate passed-in jids as active jails (as the function is documented to return -1 if the jail does not exist). This broke the jail option (in some cases?) as the jail historically hasn't needed to exist at the time of rule parsing; jids will get stored and later applied.

Fix this caller to attempt to parse *av as a number first and just use it as-is to match historical behavior. jail_getid(3) must still be used in order for name arguments to work, but it's strictly a fallback in case we weren't given a number.

[reviewer's note]
Future work should probably change the kernel interface in some way or note in the interface the caveats of the jail parameter. If given a name, it'll resolve to a jid immediately and the name gets lost in transition to kernel -- if the jail restarts and is given a different jid, the rule doesn't follow the name unless the rule's re-evaluated (which isn't done frequently, I would assume). Jids passed in aren't validated for existence and always apply to a jail with that jid, it would likely be nice if jail names worked the same way.

Test Plan

it needs to be tested, as I don't have a setup to test it with- it has only been build-tested.

Diff Detail

rS FreeBSD src repository - subversion
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ae added inline comments.
4693 ↗(On Diff #60358)

It seems *av can not be '\0' here. The NEED1() macro does check for required argument.

This revision is now accepted and ready to land.Aug 2 2019, 8:32 AM

The user who reported this on freebsd-stable@ confirms that it works for them, so I'll likely commit it tomorrow morning (~20 hours from now) with a 3-day MFC window and afterwards propose it as 11.3 EN.

4693 ↗(On Diff #60358)

Ah, true enough! I'll just drop that clause pre-commit without another review iteration of that's alright with you.

This revision was automatically updated to reflect the committed changes.