kern_jail_set() Error Scenario Question

[Matt Miller]

We ran into the following scenario in an application recently and were
wondering if the behavior of kern_jail_set() is as expected here.

This was an application bug where we were in, say, the JID 1 context
and tried to call jailparam_set() with the flags (JAIL_CREATE |
JAIL_UPDATE) and the "jid" param set to 1.  The basic idea was to
create or update JID 1 with some params, but the error was we were
already in the JID 1 context.  So, our understanding is this shouldn't
work since JID 1 already exists and you can only modify it from a
proper ancestor.

However, rather than getting an error back from jailparam_set(), it
ended up creating a second prison with JID 1, so there were two
prisons existing with JID 1 at that point.  This is based on 8.2.0
code, but, at first glance, it looks like the logic causing this may
be the same in head.

Looking at kern_jail_set(), what happens here is:

1. We find a prison with JID=1, however since it's not a proper child
we set pr = NULL in line 1024:

1011                 pr = prison_find(jid);
1012                 if (pr != NULL) {
1013                         ppr = pr->pr_parent;
1014                         /* Create: jid must not exist. */
1015                         if (cuflags == JAIL_CREATE) {
1016                                 mtx_unlock(&pr->pr_mtx);
1017                                 error = EEXIST;
1018                                 vfs_opterror(opts, "jail %d
already exists",
1019                                     jid);
1020                                 goto done_unlock_list;
1021                         }
1022                         if (!prison_ischild(mypr, pr)) {
1023                                 mtx_unlock(&pr->pr_mtx);
1024                                 pr = NULL;
1025                         } else if (pr->pr_uref == 0) {

2. Since pr is NULL, we create a new prison.  Since the jid is not
zero, we insert it in the list and set its pr_id.  At this point, we
have two prisons with a JID of 1 and the same parent prison.

1166         /* If there's no prison to update, create a new one and
link it in. */
1167         if (pr == NULL) {
...
1185                 pr = malloc(sizeof(*pr), M_PRISON, M_WAITOK | M_ZERO);
1186                 if (jid == 0) {
...
1212                 } else {
1213                         /*
1214                          * The jail already has a jid (that did
not yet exist),
1215                          * so just find where to insert it.
1216                          */
1217                         TAILQ_FOREACH(tpr, &allprison, pr_list)
1218                                 if (tpr->pr_id >= jid) {
1219                                         TAILQ_INSERT_BEFORE(tpr,
pr, pr_list);
1220                                         break;
1221                                 }
1222                 }
...
1229                 pr->pr_parent = ppr;
1230                 pr->pr_id = jid;

We wanted to see if this is per design or a situation that should
avoid creating the second prison and return an error.

Thanks,

Matt