From nobody Fri Jan 13 10:46:10 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4NtdPZ68vdz2p6B8; Fri, 13 Jan 2023 10:46:10 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4NtdPZ5jNnz3qpw; Fri, 13 Jan 2023 10:46:10 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1673606770; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Kum7N/L+Qls9RqPp32YZI3sl6Tz5Jus5R0dcs5Lj2jc=; b=MhPy3M9JQqowvRNRUe8a2AcgPGfKg9+f4ryDtR9MN8pnZRnDaWIvujrTAo3uwLwIDm66Ss Xb+3jDUp0pSLZ4oZZAjscfnhTsIc0ARey8Lgcr0qD7xQfQyN0CHKc0j/8T9yEdnuzkbDXF xF762+spGxWdPwPHDm+hGjvTAJJLfsIiDkhh/sds2xSR2pXRQEqXUl7p5PuBFqyXgxKeLw 42/R9B3fQL47LrKqV8/ztRDAw3JtW/814qMsnStZ/h59VGRVHFXzMsMm/BVd6AMxUkbOUR hvFFBghjY7EZZTDdep+0dBfqn670Q6A6lxDSc8AiaOShCpdiH31lEpw9pdcRuw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1673606770; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Kum7N/L+Qls9RqPp32YZI3sl6Tz5Jus5R0dcs5Lj2jc=; b=lcZxbVk1cKHdutwwB1ijIXrWnMZ+Pma34nKuvNl7EAONiS9v0wiBmK2OlJ+XNSRwZG+R9a +fCaX7EwffaNYOOZYAudZTN/eIx4Wjg9KS1+imzrOMIVjXFny2nh7n5WZTboAJem29l/mZ mDXMmPujK8I5qh0f53DgVvUGUekDHk5mpmplkcFRZK5YlDuBNGQk5qH1Q7iC/0OzbwC9Va ppqJmJ/Po40x7Y7iLfR5yauhqe+R8POBsubVS2JY9nHPy9ksbiPvznbg5trSNxOJK/Tq7L 1MzaTpbJMtIZFijHOgQWUDInxbIZM0x5jWZ126Nzb3Fn/6ZtW8n0d492ni4PvA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1673606770; a=rsa-sha256; cv=none; b=JiHC0YMXzK0XH/e2Zou6Mb69evSN//MSJK1aKtk8WzvK7Upnf9CJHKxeR/d7no8jpZM/Xm zmUFdRCc228+UugF1KPulB+6Z56EqQDK3u5plP9N8zDgW55tQxN1fKMRcj63Qoi6zpkAIg OZMaYv6bkhRDYDyaZZRHwmKGb/4Nw/P7XcuA31nJP39T0V8p5JghQZhXqFbZFv2EEvPCal sOgvprgAC7Py/eXIMvpP/aqpghx+GgwDhVJEGvARV2j3LOavCMRvi7U00FEUtT/rR/hyjk yl+d+FUYzk5TJikfx7/T1tKNbS6aEXibHv0Vo8nEtdCtZDiriem41OJC3O6zfw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4NtdPZ4msRz14tf; Fri, 13 Jan 2023 10:46:10 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 30DAkACY024401; Fri, 13 Jan 2023 10:46:10 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 30DAkA0F024400; Fri, 13 Jan 2023 10:46:10 GMT (envelope-from git) Date: Fri, 13 Jan 2023 10:46:10 GMT Message-Id: <202301131046.30DAkA0F024400@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Zhenlei Huang Subject: git: 8bce8d28abe6 - main - jail: Avoid multipurpose return value of function prison_ip_restrict() List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: zlei X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 8bce8d28abe658f661962a7c9d355c1e232e8371 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by zlei: URL: https://cgit.FreeBSD.org/src/commit/?id=8bce8d28abe658f661962a7c9d355c1e232e8371 commit 8bce8d28abe658f661962a7c9d355c1e232e8371 Author: Zhenlei Huang AuthorDate: 2022-12-31 02:56:58 +0000 Commit: Zhenlei Huang 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); }