d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

jail: Avoid multipurpose return value of function prison_ip_restrict()

Browse Source 
Currently function prison_ip_restrict() returns true if the replacement
buffer was used, or no buffer provided and allocation fails and should
redo. The logic is confusing and cause possibly infinite loop from
eb8dcdeac2 .

Reviewed by:	jamie, glebius
Approved by:	kp (mentor)
Differential Revision:	https://reviews.freebsd.org/D37918
This commit is contained in:
Zhenlei Huang 2022-12-31 10:56:58 +08:00
parent 89ddfbbac8
commit 8bce8d28ab
1 changed files with 29 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
sys/kern/kern_jail.c
View File

@ -777,19 +777,19 @@ prison_ip_set(struct prison *pr, const pr_family_t af, struct prison_ip *new)
/* /*
* Restrict a prison's IP address list with its parent's, possibly replacing * Restrict a prison's IP address list with its parent's, possibly replacing
* it. Return true if the replacement buffer was used (or should redo). * it. Return true if succeed, otherwise should redo.
* kern_jail_set() helper. * kern_jail_set() helper.
*/ */
static bool static bool
prison_ip_restrict(struct prison *pr, const pr_family_t af, prison_ip_restrict(struct prison *pr, const pr_family_t af,
struct prison_ip *new) struct prison_ip **newp)
{ {
const struct prison_ip *ppip = pr->pr_parent->pr_addrs[af]; const struct prison_ip *ppip = pr->pr_parent->pr_addrs[af];
const struct prison_ip *pip = pr->pr_addrs[af]; const struct prison_ip *pip = pr->pr_addrs[af];
int (*const cmp)(const void *, const void *) = pr_families[af].cmp; int (*const cmp)(const void *, const void *) = pr_families[af].cmp;
const size_t size = pr_families[af].size; const size_t size = pr_families[af].size;
struct prison_ip *new = newp != NULL ? *newp : NULL;
uint32_t ips; uint32_t ips;
bool alloced, used;
mtx_assert(&pr->pr_mtx, MA_OWNED); mtx_assert(&pr->pr_mtx, MA_OWNED);
@ -803,22 +803,21 @@ prison_ip_restrict(struct prison *pr, const pr_family_t af,
if (ppip == NULL) { if (ppip == NULL) {
if (pip != NULL) if (pip != NULL)
prison_ip_set(pr, af, NULL); prison_ip_set(pr, af, NULL);
return (false); return (true);
} }
if (!(pr->pr_flags & pr_families[af].ip_flag)) { if (!(pr->pr_flags & pr_families[af].ip_flag)) {
if (new == NULL) { if (new == NULL) {
new = prison_ip_alloc(af, ppip->ips, M_NOWAIT); new = prison_ip_alloc(af, ppip->ips, M_NOWAIT);
if (new == NULL) if (new == NULL)
return (true); /* redo */ return (false); /* Redo */
used = false; }
} else
used = true;
/* This has no user settings, so just copy the parent's list. */ /* This has no user settings, so just copy the parent's list. */
MPASS(new->ips == ppip->ips); MPASS(new->ips == ppip->ips);
bcopy(ppip + 1, new + 1, ppip->ips * size); bcopy(ppip + 1, new + 1, ppip->ips * size);
prison_ip_set(pr, af, new); prison_ip_set(pr, af, new);
return (used); if (newp != NULL)
*newp = NULL; /* Used */
} else if (pip != NULL) { } else if (pip != NULL) {
/* Remove addresses that aren't in the parent. */ /* Remove addresses that aren't in the parent. */
int i; int i;
@ -826,16 +825,10 @@ prison_ip_restrict(struct prison *pr, const pr_family_t af,
i = 0; /* index in pip */ i = 0; /* index in pip */
ips = 0; /* index in new */ ips = 0; /* index in new */
used = true;
if (new == NULL) { if (new == NULL) {
new = prison_ip_alloc(af, pip->ips, M_NOWAIT); new = prison_ip_alloc(af, pip->ips, M_NOWAIT);
if (new == NULL) if (new == NULL)
return (true); /* redo */ return (false); /* Redo */
used = false;
alloced = true;
} else {
used = true;
alloced = false;
} }
for (int pi = 0; pi < ppip->ips; pi++) for (int pi = 0; pi < ppip->ips; pi++)
@ -873,10 +866,9 @@ prison_ip_restrict(struct prison *pr, const pr_family_t af,
} }
} }
if (ips == 0) { if (ips == 0) {
if (alloced) if (newp == NULL || *newp == NULL)
prison_ip_free(new); prison_ip_free(new);
new = NULL; new = NULL;
used = false;
} else { } else {
/* Shrink to real size */ /* Shrink to real size */
KASSERT((new->ips >= ips), KASSERT((new->ips >= ips),
@ -884,9 +876,10 @@ prison_ip_restrict(struct prison *pr, const pr_family_t af,
new->ips = ips; new->ips = ips;
} }
prison_ip_set(pr, af, new); prison_ip_set(pr, af, new);
return (used); if (newp != NULL)
*newp = NULL; /* Used */
} }
return (false); return (true);
} }
/* /*
@ -984,10 +977,12 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
int jid, jsys, len, level; int jid, jsys, len, level;
int childmax, osreldt, rsnum, slevel; int childmax, osreldt, rsnum, slevel;
#ifdef INET #ifdef INET
int ip4s, redo_ip4; int ip4s;
bool redo_ip4;
#endif #endif
#ifdef INET6 #ifdef INET6
int ip6s, redo_ip6; int ip6s;
bool redo_ip6;
#endif #endif
uint64_t pr_allow, ch_allow, pr_flags, ch_flags; uint64_t pr_allow, ch_allow, pr_flags, ch_flags;
uint64_t pr_allow_diff; uint64_t pr_allow_diff;
@ -1864,7 +1859,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
/* Set the parameters of the prison. */ /* Set the parameters of the prison. */
#ifdef INET #ifdef INET
redo_ip4 = 0; redo_ip4 = false;
if (pr_flags & PR_IP4_USER) { if (pr_flags & PR_IP4_USER) {
pr->pr_flags |= PR_IP4; pr->pr_flags |= PR_IP4;
prison_ip_set(pr, PR_INET, ip4); prison_ip_set(pr, PR_INET, ip4);
@ -1876,15 +1871,15 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
continue; continue;
} }
#endif #endif
if (prison_ip_restrict(tpr, PR_INET, NULL)) { if (!prison_ip_restrict(tpr, PR_INET, NULL)) {
redo_ip4 = 1; redo_ip4 = true;
descend = 0; descend = 0;
} }
} }
} }
#endif #endif
#ifdef INET6 #ifdef INET6
redo_ip6 = 0; redo_ip6 = false;
if (pr_flags & PR_IP6_USER) { if (pr_flags & PR_IP6_USER) {
pr->pr_flags |= PR_IP6; pr->pr_flags |= PR_IP6;
prison_ip_set(pr, PR_INET6, ip6); prison_ip_set(pr, PR_INET6, ip6);
@ -1896,8 +1891,8 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
continue; continue;
} }
#endif #endif
if (prison_ip_restrict(tpr, PR_INET6, NULL)) { if (!prison_ip_restrict(tpr, PR_INET6, NULL)) {
redo_ip6 = 1; redo_ip6 = true;
descend = 0; descend = 0;
} }
} }
@ -2041,9 +2036,10 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
#ifdef INET #ifdef INET
while (redo_ip4) { while (redo_ip4) {
ip4s = pr->pr_addrs[PR_INET]->ips; ip4s = pr->pr_addrs[PR_INET]->ips;
MPASS(ip4 == NULL);
ip4 = prison_ip_alloc(PR_INET, ip4s, M_WAITOK); ip4 = prison_ip_alloc(PR_INET, ip4s, M_WAITOK);
mtx_lock(&pr->pr_mtx); mtx_lock(&pr->pr_mtx);
redo_ip4 = 0; redo_ip4 = false;
FOREACH_PRISON_DESCENDANT_LOCKED(pr, tpr, descend) { FOREACH_PRISON_DESCENDANT_LOCKED(pr, tpr, descend) {
#ifdef VIMAGE #ifdef VIMAGE
if (tpr->pr_flags & PR_VNET) { if (tpr->pr_flags & PR_VNET) {
@ -2051,12 +2047,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
continue; continue;
} }
#endif #endif
if (prison_ip_restrict(tpr, PR_INET, ip4)) { redo_ip4 = !prison_ip_restrict(tpr, PR_INET, &ip4);
if (ip4 != NULL)
ip4 = NULL;
else
redo_ip4 = 1;
}
} }
mtx_unlock(&pr->pr_mtx); mtx_unlock(&pr->pr_mtx);
} }
@ -2064,9 +2055,10 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
#ifdef INET6 #ifdef INET6
while (redo_ip6) { while (redo_ip6) {
ip6s = pr->pr_addrs[PR_INET6]->ips; ip6s = pr->pr_addrs[PR_INET6]->ips;
MPASS(ip6 == NULL);
ip6 = prison_ip_alloc(PR_INET6, ip6s, M_WAITOK); ip6 = prison_ip_alloc(PR_INET6, ip6s, M_WAITOK);
mtx_lock(&pr->pr_mtx); mtx_lock(&pr->pr_mtx);
redo_ip6 = 0; redo_ip6 = false;
FOREACH_PRISON_DESCENDANT_LOCKED(pr, tpr, descend) { FOREACH_PRISON_DESCENDANT_LOCKED(pr, tpr, descend) {
#ifdef VIMAGE #ifdef VIMAGE
if (tpr->pr_flags & PR_VNET) { if (tpr->pr_flags & PR_VNET) {
@ -2074,12 +2066,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
continue; continue;
} }
#endif #endif
if (prison_ip_restrict(tpr, PR_INET6, ip6)) { redo_ip6 = !prison_ip_restrict(tpr, PR_INET6, &ip6);
if (ip6 != NULL)
ip6 = NULL;
else
redo_ip6 = 1;
}
} }
mtx_unlock(&pr->pr_mtx); mtx_unlock(&pr->pr_mtx);
} }