Date: Wed, 3 Oct 2018 15:51:33 +0000 From: Brooks Davis <brooks@freebsd.org> To: Rick Macklem <rmacklem@uoguelph.ca> Cc: "freebsd-current@FreeBSD.org" <freebsd-current@FreeBSD.org>, Josh Paetzel <josh@tcbug.org> Subject: Re: which way to update export_args structure? Message-ID: <20181003155133.GA57729@spindle.one-eyed-alien.net> In-Reply-To: <YTOPR0101MB182021549F8CF8277477A4C5DDE90@YTOPR0101MB1820.CANPRD01.PROD.OUTLOOK.COM> References: <YTOPR0101MB182021549F8CF8277477A4C5DDE90@YTOPR0101MB1820.CANPRD01.PROD.OUTLOOK.COM>
next in thread | previous in thread | raw e-mail | index | archive | help
--qMm9M+Fa2AknHoGS
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Wed, Oct 03, 2018 at 12:40:27AM +0000, Rick Macklem wrote:
> Hi,
>=20
> I am working on updating "struct export_args" to fix/add a few things.
> One of these is that "ex_flags" is an int, but the flags are defined in m=
ount.h
> as MNT_xx bits that now exceed 32bits (mnt_flag is now uint64_t).
> For now, this doesn't break anything, since the flags used by ex_flags are
> all defined in the low order 32bits but...it seems like this should be ad=
dressed
> by a new version of "struct export_args".
>=20
> I have two versions of the updated structure:
> A)
> struct export_args {
> uint64_t ex_flags; /* export related flags */
> uid_t ex_root; /* mapping for root uid */
> struct xucred ex_anon; /* mapping for anonymous user */
> struct sockaddr *ex_addr; /* net address to which exported */
> u_char ex_addrlen; /* and the net address length */
> struct sockaddr *ex_mask; /* mask of valid bits in saddr */
> u_char ex_masklen; /* and the smask length */
> char *ex_indexfile; /* index file for WebNFS URLs */
> int ex_numsecflavors; /* security flavor count */
> int ex_secflavors[MAXSECFLAVORS]; /* list of security flavors */
> int32_t ex_fsid; /* mnt_stat.f_fsid.val[0] if */
> /* MNT_EXPORTFSID set in ex_flags64 */
> gid_t *ex_suppgroups; /* Supplemental groups if */
> /* ex_anon.cr_ngroups > XU_NGROUPS */
> };
> B)
> struct export_args {
> int ex_flags; /* export related flags */
> uid_t ex_root; /* mapping for root uid */
> struct xucred ex_anon; /* mapping for anonymous user */
> struct sockaddr *ex_addr; /* net address to which exported */
> u_char ex_addrlen; /* and the net address length */
> struct sockaddr *ex_mask; /* mask of valid bits in saddr */
> u_char ex_masklen; /* and the smask length */
> char *ex_indexfile; /* index file for WebNFS URLs */
> int ex_numsecflavors; /* security flavor count */
> int ex_secflavors[MAXSECFLAVORS]; /* list of security flavors */
> uint64_t ex_flagshighbits; /* High order bits of mnt_flag */
> int32_t ex_fsid; /* mnt_stat.f_fsid.val[0] if */
> /* MNT_EXPORTFSID set in ex_flags64 */
> gid_t *ex_suppgroups; /* Supplemental groups if */
> /* ex_anon.cr_ngroups > XU_NGROUPS */
> };
>=20
> A) does the obvious thing. Unfortunately, this changes the vfs KABI
> (specifically the function vfs_oexport_conv()) such that a file system
> module compiled with an unpatched mount.h could crash a patched system.
> As such, I think it couldn't be MFC'd and would be stuck in head/current
> until FreeBSD13 (or FreeBSD14 if 13 gets skipped over;-).
>=20
> B) doesn't change any fields, but adds a second ex_flagshighbits for the =
high
> order bit. Since it only adds fields where none of those bits are used af=
ter
> the exports are processed by vfs_export() and, as such, will not break
> the VFS KABI, since vfs_domount_update() differentiates which version
> of export_args is being used.
> As such, I believe this version can be MFC'd. However, it does seem confu=
sing
> to have the two ex_flags fields for the low and high 32bits.
I see you've found a way to do compatibility for a new ABI. If you
wanted to avoid changing the struct size, there is 3 bytes of usable
padding after each ex_addrlen and ex_masklen.
One general question: why does export_args still exist as an interface
between userspace and the kernel? It's passed via nmount so it seems
like the individual entries should be elements in the vector instead.
This would be much friendlier if one wanted to do 32-bit compat support
for mountd.
-- Brooks
--qMm9M+Fa2AknHoGS
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
iQEcBAEBAgAGBQJbtOWEAAoJEKzQXbSebgfA+jEH/iEJa8hdXopLWA8ECi5CHeoN
qg+T05dBwCFkp0u87n2+rq+GWnxYho8y+Hh3orqF2p4l4KcG/f7TMo/L56nKBVyz
jrPS1If9vBJEPqwyEzaKQAYrxr28v8o6uovaKH1zcWvQbhIJzBWRG5RICgPySMtF
FQynUHa0xLxEg3OgBD79TOPTuJc4IeeI94veKDF6eAezHgXuba5OTi6lIlGi+Lgh
eg2QXaaUkri+lOcpt3Qbhm8S54/DFPxlVxzaihgwACNPUsybjkOcK2TBh02ItlBc
yuFkzYbIRstHiDLle3IRHaUmrRHbcmqh6MNraXiKLNOoo1ZlGSCqbRR3DRVb7kI=
=ZMKq
-----END PGP SIGNATURE-----
--qMm9M+Fa2AknHoGS--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20181003155133.GA57729>