On 08/22/13 17:46, Matt Miller wrote:
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.
That's definitely not per design. I'll try reproducing this, and put in
correct logic. The proper response is indeed an error: ENOENT, because
from inside the JID 1 context you shouldn't be able to see jail #1 (you
can't operate on your own jail).
- Jamie
___
freebsd-questions@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-questions
To unsubscribe, send any mail to freebsd-questions-unsubscr...@freebsd.org
