git: 8bce8d28abe6 - main - jail: Avoid multipurpose return value of function prison_ip_restrict()
Date: Fri, 13 Jan 2023 10:46:10 UTC
The branch main has been updated by zlei:
URL: https://cgit.FreeBSD.org/src/commit/?id=8bce8d28abe658f661962a7c9d355c1e232e8371
commit 8bce8d28abe658f661962a7c9d355c1e232e8371
Author: Zhenlei Huang <zlei@FreeBSD.org>
AuthorDate: 2022-12-31 02:56:58 +0000
Commit: Zhenlei Huang <zlei@FreeBSD.org>
CommitDate: 2023-01-13 10:45:14 +0000
jail: Avoid multipurpose return value of function prison_ip_restrict()
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
eb8dcdeac22d .
Reviewed by: jamie, glebius
Approved by: kp (mentor)
Differential Revision: https://reviews.freebsd.org/D37918
---
sys/kern/kern_jail.c | 71 +++++++++++++++++++++-------------------------------
1 file changed, 29 insertions(+), 42 deletions(-)
diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
index e9fc8ddae144..0820de9d5ac0 100644
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -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
- * it. Return true if the replacement buffer was used (or should redo).
+ * it. Return true if succeed, otherwise should redo.
* kern_jail_set() helper.
*/
static bool
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 *pip = pr->pr_addrs[af];
int (*const cmp)(const void *, const void *) = pr_families[af].cmp;
const size_t size = pr_families[af].size;
+ struct prison_ip *new = newp != NULL ? *newp : NULL;
uint32_t ips;
- bool alloced, used;
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 (pip != NULL)
prison_ip_set(pr, af, NULL);
- return (false);
+ return (true);
}
if (!(pr->pr_flags & pr_families[af].ip_flag)) {
if (new == NULL) {
new = prison_ip_alloc(af, ppip->ips, M_NOWAIT);
if (new == NULL)
- return (true); /* redo */
- used = false;
- } else
- used = true;
+ return (false); /* Redo */
+ }
/* This has no user settings, so just copy the parent's list. */
MPASS(new->ips == ppip->ips);
bcopy(ppip + 1, new + 1, ppip->ips * size);
prison_ip_set(pr, af, new);
- return (used);
+ if (newp != NULL)
+ *newp = NULL; /* Used */
} else if (pip != NULL) {
/* Remove addresses that aren't in the parent. */
int i;
@@ -826,16 +825,10 @@ prison_ip_restrict(struct prison *pr, const pr_family_t af,
i = 0; /* index in pip */
ips = 0; /* index in new */
- used = true;
if (new == NULL) {
new = prison_ip_alloc(af, pip->ips, M_NOWAIT);
if (new == NULL)
- return (true); /* redo */
- used = false;
- alloced = true;
- } else {
- used = true;
- alloced = false;
+ return (false); /* Redo */
}
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 (alloced)
+ if (newp == NULL || *newp == NULL)
prison_ip_free(new);
new = NULL;
- used = false;
} else {
/* Shrink to real size */
KASSERT((new->ips >= ips),
@@ -884,9 +876,10 @@ prison_ip_restrict(struct prison *pr, const pr_family_t af,
new->ips = ips;
}
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 childmax, osreldt, rsnum, slevel;
#ifdef INET
- int ip4s, redo_ip4;
+ int ip4s;
+ bool redo_ip4;
#endif
#ifdef INET6
- int ip6s, redo_ip6;
+ int ip6s;
+ bool redo_ip6;
#endif
uint64_t pr_allow, ch_allow, pr_flags, ch_flags;
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. */
#ifdef INET
- redo_ip4 = 0;
+ redo_ip4 = false;
if (pr_flags & PR_IP4_USER) {
pr->pr_flags |= PR_IP4;
prison_ip_set(pr, PR_INET, ip4);
@@ -1876,15 +1871,15 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
continue;
}
#endif
- if (prison_ip_restrict(tpr, PR_INET, NULL)) {
- redo_ip4 = 1;
+ if (!prison_ip_restrict(tpr, PR_INET, NULL)) {
+ redo_ip4 = true;
descend = 0;
}
}
}
#endif
#ifdef INET6
- redo_ip6 = 0;
+ redo_ip6 = false;
if (pr_flags & PR_IP6_USER) {
pr->pr_flags |= PR_IP6;
prison_ip_set(pr, PR_INET6, ip6);
@@ -1896,8 +1891,8 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
continue;
}
#endif
- if (prison_ip_restrict(tpr, PR_INET6, NULL)) {
- redo_ip6 = 1;
+ if (!prison_ip_restrict(tpr, PR_INET6, NULL)) {
+ redo_ip6 = true;
descend = 0;
}
}
@@ -2041,9 +2036,10 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
#ifdef INET
while (redo_ip4) {
ip4s = pr->pr_addrs[PR_INET]->ips;
+ MPASS(ip4 == NULL);
ip4 = prison_ip_alloc(PR_INET, ip4s, M_WAITOK);
mtx_lock(&pr->pr_mtx);
- redo_ip4 = 0;
+ redo_ip4 = false;
FOREACH_PRISON_DESCENDANT_LOCKED(pr, tpr, descend) {
#ifdef VIMAGE
if (tpr->pr_flags & PR_VNET) {
@@ -2051,12 +2047,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
continue;
}
#endif
- if (prison_ip_restrict(tpr, PR_INET, ip4)) {
- if (ip4 != NULL)
- ip4 = NULL;
- else
- redo_ip4 = 1;
- }
+ redo_ip4 = !prison_ip_restrict(tpr, PR_INET, &ip4);
}
mtx_unlock(&pr->pr_mtx);
}
@@ -2064,9 +2055,10 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
#ifdef INET6
while (redo_ip6) {
ip6s = pr->pr_addrs[PR_INET6]->ips;
+ MPASS(ip6 == NULL);
ip6 = prison_ip_alloc(PR_INET6, ip6s, M_WAITOK);
mtx_lock(&pr->pr_mtx);
- redo_ip6 = 0;
+ redo_ip6 = false;
FOREACH_PRISON_DESCENDANT_LOCKED(pr, tpr, descend) {
#ifdef VIMAGE
if (tpr->pr_flags & PR_VNET) {
@@ -2074,12 +2066,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
continue;
}
#endif
- if (prison_ip_restrict(tpr, PR_INET6, ip6)) {
- if (ip6 != NULL)
- ip6 = NULL;
- else
- redo_ip6 = 1;
- }
+ redo_ip6 = !prison_ip_restrict(tpr, PR_INET6, &ip6);
}
mtx_unlock(&pr->pr_mtx);
}