Date: Fri, 3 Feb 2023 18:11:25 +0800 From: Zhenlei Huang <zlei@FreeBSD.org> To: Gleb Smirnoff <glebius@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 8bce8d28abe6 - main - jail: Avoid multipurpose return value of function prison_ip_restrict() Message-ID: <DFF01C89-0B7C-40E2-AA6D-9B794A759E03@FreeBSD.org> In-Reply-To: <Y8GzjNMleGOLchR6@FreeBSD.org> References: <202301131046.30DAkA0F024400@gitrepo.freebsd.org> <Y8GzjNMleGOLchR6@FreeBSD.org>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
Hi Gleb,
> On Jan 14, 2023, at 3:39 AM, Gleb Smirnoff <glebius@freebsd.org> wrote:
>
> Zhenlei,
>
> a couple concise assignments missed:
>
> Z> @@ -1876,15 +1871,15 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
> Z> continue;
> Z> }
> Z> #endif
> Z> - if (prison_ip_restrict(tpr, PR_INET, NULL)) {
> Z> - redo_ip4 = 1;
> Z> + if (!prison_ip_restrict(tpr, PR_INET, NULL)) {
> Z> + redo_ip4 = true;
> Z> descend = 0;
> Z> }
> Z> }
> Z> }
>
> redo_ip4 = !prison_ip_restrict(tpr, PR_INET, NULL);
I think that is wrong, as `prison_ip_restrict` is called in loop round.
`redo_ip4` might flip to false on next round.
So the previous logic is right.
> + redo_ip4 = !prison_ip_restrict(tpr, PR_INET, &ip4);
Should be `redo_ip4 |= !prison_ip_restrict(tpr, PR_INET, &ip4);`
>
> Z> @@ -1896,8 +1891,8 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
> Z> continue;
> Z> }
> Z> #endif
> Z> - if (prison_ip_restrict(tpr, PR_INET6, NULL)) {
> Z> - redo_ip6 = 1;
> Z> + if (!prison_ip_restrict(tpr, PR_INET6, NULL)) {
> Z> + redo_ip6 = true;
> Z> descend = 0;
> Z> }
> Z> }
>
> redo_ip6 = !prison_ip_restrict(tpr, PR_INET6, NULL);
>
> --
> Gleb Smirnoff
PS, the logic redo_ip4 / redo_ip6 under low memory pressure can be optimized and I'll do that later.
Best regards,
Zhenlei
[-- Attachment #2 --]
<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi Gleb,<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Jan 14, 2023, at 3:39 AM, Gleb Smirnoff <<a href="mailto:glebius@freebsd.org" class="">glebius@freebsd.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class=""> Zhenlei,<br class=""><br class="">a couple concise assignments missed:<br class=""><br class="">Z> @@ -1876,15 +1871,15 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>continue;<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">Z> #endif<br class="">Z> -<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (prison_ip_restrict(tpr, PR_INET, NULL)) {<br class="">Z> -<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>redo_ip4 = 1;<br class="">Z> +<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (!prison_ip_restrict(tpr, PR_INET, NULL)) {<br class="">Z> +<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>redo_ip4 = true;<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>descend = 0;<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>redo_ip4 = !prison_ip_restrict(tpr, PR_INET, NULL);<br class=""></div></div></blockquote><div><br class=""></div><div>I think that is wrong, as `prison_ip_restrict` is called in loop round.</div><div>`redo_ip4` might flip to false on next round.</div><div><br class=""></div><div>So the previous logic is right.</div><div><br class=""></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Menlo-Regular;" class="">> +</span> <span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Menlo-Regular;" class="">redo_ip4 = !prison_ip_restrict(tpr, PR_INET, &ip4);</span></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Menlo-Regular;" class=""><br class=""></span></div><div><font color="#000000" face="Menlo-Regular" class=""><span style="caret-color: rgb(0, 0, 0);" class="">Should be `redo_ip4 |= </span></font><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Menlo-Regular;" class="">!prison_ip_restrict(tpr, PR_INET, &ip4);`</span></div><div><br class=""></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Menlo-Regular;" class=""><br class=""></span></div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">Z> @@ -1896,8 +1891,8 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>continue;<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">Z> #endif<br class="">Z> -<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (prison_ip_restrict(tpr, PR_INET6, NULL)) {<br class="">Z> -<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>redo_ip6 = 1;<br class="">Z> +<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (!prison_ip_restrict(tpr, PR_INET6, NULL)) {<br class="">Z> +<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>redo_ip6 = true;<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>descend = 0;<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">Z> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>redo_ip6 = !prison_ip_restrict(tpr, PR_INET6, NULL);<br class=""><br class="">-- <br class="">Gleb Smirnoff<br class=""></div></div></blockquote><br class=""></div><div><br class=""></div><div>PS, the logic redo_ip4 / redo_ip6 under low memory pressure can be optimized and I'll do that later.</div><br class=""><div class="">
<div>Best regards,</div><div>Zhenlei</div>
</div>
<br class=""></body></html>
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?DFF01C89-0B7C-40E2-AA6D-9B794A759E03>