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>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_281FF14D-CED0-49A3-816F-155DC72D3787
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
charset=us-ascii
Hi Gleb,
> On Jan 14, 2023, at 3:39 AM, Gleb Smirnoff <glebius@freebsd.org> =
wrote:
>=20
> Zhenlei,
>=20
> a couple concise assignments missed:
>=20
> 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 =3D 1;
> Z> + if (!prison_ip_restrict(tpr, PR_INET, NULL)) {
> Z> + redo_ip4 =3D true;
> Z> descend =3D 0;
> Z> }
> Z> }
> Z> }
>=20
> redo_ip4 =3D !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 =3D !prison_ip_restrict(tpr, PR_INET, &ip4);
Should be `redo_ip4 |=3D !prison_ip_restrict(tpr, PR_INET, &ip4);`
>=20
> 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 =3D 1;
> Z> + if (!prison_ip_restrict(tpr, PR_INET6, NULL)) {
> Z> + redo_ip6 =3D true;
> Z> descend =3D 0;
> Z> }
> Z> }
>=20
> redo_ip6 =3D !prison_ip_restrict(tpr, PR_INET6, NULL);
>=20
> --=20
> 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
--Apple-Mail=_281FF14D-CED0-49A3-816F-155DC72D3787
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html;
charset=us-ascii
<html><head><meta http-equiv=3D"Content-Type" content=3D"text/html; =
charset=3Dus-ascii"></head><body style=3D"word-wrap: break-word; =
-webkit-nbsp-mode: space; line-break: after-white-space;" class=3D"">Hi =
Gleb,<br class=3D""><div><br class=3D""><blockquote type=3D"cite" =
class=3D""><div class=3D"">On Jan 14, 2023, at 3:39 AM, Gleb Smirnoff =
<<a href=3D"mailto:glebius@freebsd.org" =
class=3D"">glebius@freebsd.org</a>> wrote:</div><br =
class=3D"Apple-interchange-newline"><div class=3D""><div class=3D""> =
Zhenlei,<br class=3D""><br class=3D"">a couple concise assignments =
missed:<br class=3D""><br class=3D"">Z> @@ -1876,15 +1871,15 @@ =
kern_jail_set(struct thread *td, struct uio *optuio, int flags)<br =
class=3D"">Z> <span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>continue;<br class=3D"">Z> =
<span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span>}<br class=3D"">Z> #endif<br class=3D"">Z> -<span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>if =
(prison_ip_restrict(tpr, PR_INET, NULL)) {<br class=3D"">Z> -<span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>redo_ip4 =
=3D 1;<br class=3D"">Z> +<span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>if (!prison_ip_restrict(tpr, =
PR_INET, NULL)) {<br class=3D"">Z> +<span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>redo_ip4 =3D true;<br =
class=3D"">Z> <span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>descend =3D 0;<br class=3D"">Z> =
<span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span>}<br class=3D"">Z> <span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>}<br class=3D"">Z> <span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>}<br =
class=3D""><br class=3D""><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>redo_ip4 =3D =
!prison_ip_restrict(tpr, PR_INET, NULL);<br =
class=3D""></div></div></blockquote><div><br class=3D""></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=3D""></div><div>So the previous logic is =
right.</div><div><br class=3D""></div><div><span style=3D"caret-color: =
rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Menlo-Regular;" =
class=3D"">> +</span> <span style=3D"caret-color: =
rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Menlo-Regular;" =
class=3D"">redo_ip4 =3D !prison_ip_restrict(tpr, PR_INET, =
&ip4);</span></div><div><span style=3D"caret-color: rgb(0, 0, 0); =
color: rgb(0, 0, 0); font-family: Menlo-Regular;" class=3D""><br =
class=3D""></span></div><div><font color=3D"#000000" =
face=3D"Menlo-Regular" class=3D""><span style=3D"caret-color: rgb(0, 0, =
0);" class=3D"">Should be `redo_ip4 |=3D </span></font><span =
style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: =
Menlo-Regular;" class=3D"">!prison_ip_restrict(tpr, PR_INET, =
&ip4);`</span></div><div><br class=3D""></div><div><span =
style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: =
Menlo-Regular;" class=3D""><br class=3D""></span></div><br =
class=3D""><blockquote type=3D"cite" class=3D""><div class=3D""><div =
class=3D""><br class=3D"">Z> @@ -1896,8 +1891,8 @@ =
kern_jail_set(struct thread *td, struct uio *optuio, int flags)<br =
class=3D"">Z> <span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>continue;<br class=3D"">Z> =
<span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span>}<br class=3D"">Z> #endif<br class=3D"">Z> -<span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>if =
(prison_ip_restrict(tpr, PR_INET6, NULL)) {<br class=3D"">Z> -<span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>redo_ip6 =
=3D 1;<br class=3D"">Z> +<span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>if (!prison_ip_restrict(tpr, =
PR_INET6, NULL)) {<br class=3D"">Z> +<span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>redo_ip6 =3D true;<br =
class=3D"">Z> <span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>descend =3D 0;<br class=3D"">Z> =
<span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span>}<br class=3D"">Z> <span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre"> </span>}<br class=3D""><br =
class=3D""><span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span>redo_ip6 =3D !prison_ip_restrict(tpr, PR_INET6, NULL);<br =
class=3D""><br class=3D"">-- <br class=3D"">Gleb Smirnoff<br =
class=3D""></div></div></blockquote><br class=3D""></div><div><br =
class=3D""></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=3D""><div=
class=3D"">
<div>Best regards,</div><div>Zhenlei</div>
</div>
<br class=3D""></body></html>=
--Apple-Mail=_281FF14D-CED0-49A3-816F-155DC72D3787--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?DFF01C89-0B7C-40E2-AA6D-9B794A759E03>