Date: Tue, 13 Dec 2022 14:19:39 -0800 From: Rick Macklem <rick.macklem@gmail.com> To: FreeBSD CURRENT <freebsd-current@freebsd.org> Subject: What to do about a few lines in vfs_domount() never executed? Message-ID: <CAM5tNy7DM__tj4CQ0L_-ugo5krmEvLJgbU-WFLs47MrRBuGaNQ@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
--000000000000d6a85d05efbd0464 Content-Type: text/plain; charset="UTF-8" Hi, While working on getting mountd/nfsd to run in a vnet prison, I came across the following lines near the beginning of vfs_domount() in sys/kern/vfs_mount.c: if (fsflags & MNT_EXPORTED) { error = priv_check(td, PRIV_VFS_MOUNT_EXPORTED); if (error) return (error); } #1 - Since MNT_EXPORTED is never set in fsflags, this code never gets executed. --> I am asking what to do with the above code, since that changes for the patch that allows mountd to run in a vnet prison. #2 - priv_check(td, PRIV_VFS_MOUNT_EXPORTED) always returns 0 because nothing in sys/kern/kern_priv.c checks PRIV_VFS_MOUNT_EXPORTED. I don't know what the original author's thinking was w.r.t. this. Setting exports already checks that the mount operation can be done by the requestor. So, what do you think should be done with the above code snippet? - Consider it cruft and delete it. - Try and figure out what PRIV_VFS_MOUNT_EXPORTED should check? - Leave it as is. After the patch that allows mountd to run in a vnet prison, MNT_EXPORTED will be set in fsflags, but the priv_check() call will just return 0. (A little overhead, but otherwise no semantics change.) rick --000000000000d6a85d05efbd0464 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><div class=3D"gmail_default" style=3D"fon= t-family:monospace">Hi,</div><div class=3D"gmail_default" style=3D"font-fam= ily:monospace"><br></div><div class=3D"gmail_default" style=3D"font-family:= monospace">While working on getting mountd/nfsd to run in a vnet</div><div = class=3D"gmail_default" style=3D"font-family:monospace">prison, I came acro= ss the following lines near the</div><div class=3D"gmail_default" style=3D"= font-family:monospace">beginning of vfs_domount() in sys/kern/vfs_mount.c:<= /div><div class=3D"gmail_default" style=3D"font-family:monospace"><br></div= ><div class=3D"gmail_default" style=3D"font-family:monospace">if (fsflags &= amp; MNT_EXPORTED) {</div><div class=3D"gmail_default" style=3D"font-family= :monospace">=C2=A0 =C2=A0 =C2=A0error =3D priv_check(td, PRIV_VFS_MOUNT_EXP= ORTED);</div><div class=3D"gmail_default" style=3D"font-family:monospace">= =C2=A0 =C2=A0 =C2=A0if (error)</div><div class=3D"gmail_default" style=3D"f= ont-family:monospace">=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return (erro= r);</div><div class=3D"gmail_default" style=3D"font-family:monospace">}</di= v><div class=3D"gmail_default" style=3D"font-family:monospace"><br></div><d= iv class=3D"gmail_default" style=3D"font-family:monospace">#1 - Since MNT_E= XPORTED is never set in fsflags, this code never</div><div class=3D"gmail_d= efault" style=3D"font-family:monospace">=C2=A0 =C2=A0 =C2=A0gets executed.<= /div><div class=3D"gmail_default" style=3D"font-family:monospace">=C2=A0 = =C2=A0 =C2=A0--> I am asking what to do with the above code, since that<= /div><div class=3D"gmail_default" style=3D"font-family:monospace">=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0changes for the patch that allows mountd to run = in a vnet</div><div class=3D"gmail_default" style=3D"font-family:monospace"= >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0prison.</div><div class=3D"gmail_default= " style=3D"font-family:monospace">#2 - priv_check(td, PRIV_VFS_MOUNT_EXPORT= ED) always returns 0</div><div class=3D"gmail_default" style=3D"font-family= :monospace">=C2=A0 =C2=A0 =C2=A0because nothing in sys/kern/kern_priv.c che= cks</div><div class=3D"gmail_default" style=3D"font-family:monospace">=C2= =A0 =C2=A0 =C2=A0PRIV_VFS_MOUNT_EXPORTED.</div><div class=3D"gmail_default"= style=3D"font-family:monospace"><br></div><div class=3D"gmail_default" sty= le=3D"font-family:monospace">I don't know what the original author'= s thinking was w.r.t. this.</div><div class=3D"gmail_default" style=3D"font= -family:monospace">Setting exports already checks that the mount operation = can be</div><div class=3D"gmail_default" style=3D"font-family:monospace">do= ne by the requestor.</div><div class=3D"gmail_default" style=3D"font-family= :monospace"><br></div><div class=3D"gmail_default" style=3D"font-family:mon= ospace">So, what do you think should be done with the above code snippet?</= div><div class=3D"gmail_default" style=3D"font-family:monospace">- Consider= it cruft and delete it.</div><div class=3D"gmail_default" style=3D"font-fa= mily:monospace">- Try and figure out what PRIV_VFS_MOUNT_EXPORTED should ch= eck?</div><div class=3D"gmail_default" style=3D"font-family:monospace">- Le= ave it as is. After the patch that allows mountd to run in</div><div class= =3D"gmail_default" style=3D"font-family:monospace">=C2=A0 a vnet prison, MN= T_EXPORTED will be set in fsflags, but the</div><div class=3D"gmail_default= " style=3D"font-family:monospace">=C2=A0 priv_check() call will just return= 0. (A little overhead,</div><div class=3D"gmail_default" style=3D"font-fam= ily:monospace">=C2=A0 but otherwise no semantics change.)</div><div class= =3D"gmail_default" style=3D"font-family:monospace"><br></div><div class=3D"= gmail_default" style=3D"font-family:monospace">rick</div><div class=3D"gmai= l_default" style=3D"font-family:monospace"><br></div></div></div> --000000000000d6a85d05efbd0464--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAM5tNy7DM__tj4CQ0L_-ugo5krmEvLJgbU-WFLs47MrRBuGaNQ>