Date: Sat, 20 Oct 2018 01:17:37 +0000 From: Rick Macklem <rmacklem@uoguelph.ca> To: Brooks Davis <brooks@freebsd.org>, FreeBSD Current <freebsd-current@freebsd.org>, Josh Paetzel <josh@tcbug.org> Subject: Re: which way to update export_args structure? Message-ID: <YTOPR0101MB11626B32F73B520FBDA3C633DDFA0@YTOPR0101MB1162.CANPRD01.PROD.OUTLOOK.COM> In-Reply-To: <20181008170428.GB9766@spindle.one-eyed-alien.net> References: <YTOPR0101MB182021549F8CF8277477A4C5DDE90@YTOPR0101MB1820.CANPRD01.PROD.OUTLOOK.COM> <20181003155133.GA57729@spindle.one-eyed-alien.net> <YTOPR0101MB18207FF98DED0232B9BB1B4FDDE50@YTOPR0101MB1820.CANPRD01.PROD.OUTLOOK.COM>, <20181008170428.GB9766@spindle.one-eyed-alien.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Brooks Davis wrote:
> Yes, I think that's the right way foward. Thanks for following up.
>Rick Macklem wrote:
>> Just in case you missed it in the email thread, in your general question=
below...
>> Did you mean/suggest that the fields of "struct export_args" be passed i=
n as
>> separate options to nmount(2)?
>>
>> This sounds like a reasonable idea to me and I can ping Josh Paetzel w.r=
.t. the
>> changes to mountd.c to do it. (We are still in the testing stage for the=
updated
>> struct, so we might as well get that working first.)
>>
Well, Josh and I now have the code working via. passing the export_args
structure into the kernel using the "export" nmount(2) option.
I have coded a partial patch (not complete nor tested) to pass the fields i=
n as
separate nmount(2) arguments. Since the patch has gotten fairly large
already, I wanted to see if people do think this is the correct approach.
(I'll admit I don't understand why having the arguments would matter, given
that only mountd does it. Would anyone run a 32bit mountd on a 64bit kerne=
l?)
Anyhow, here's the partial patch showing the main changes when going from
passing in "struct export_args" to passing in separate fields:
--- kern/vfs_mount.c.nofsid2 2018-10-16 23:45:33.540348000 -0400
+++ kern/vfs_mount.c 2018-10-19 20:01:14.927370000 -0400
@@ -277,6 +277,7 @@ vfs_buildopts(struct uio *auio, struct v
size_t memused, namelen, optlen;
unsigned int i, iovcnt;
int error;
+ char *cp;
=20
opts =3D malloc(sizeof(struct vfsoptlist), M_MOUNT, M_WAITOK);
TAILQ_INIT(opts);
@@ -325,7 +326,7 @@ vfs_buildopts(struct uio *auio, struct v
}
if (optlen !=3D 0) {
opt->len =3D optlen;
- opt->value =3D malloc(optlen, M_MOUNT, M_WAITOK);
+ opt->value =3D malloc(optlen + 1, M_MOUNT, M_WAITOK);
if (auio->uio_segflg =3D=3D UIO_SYSSPACE) {
bcopy(auio->uio_iov[i + 1].iov_base, opt->value,
optlen);
@@ -335,6 +336,8 @@ vfs_buildopts(struct uio *auio, struct v
if (error)
goto bad;
}
+ cp =3D (char *)opt->value;
+ cp[optlen] =3D '\0';
}
}
vfs_sanitizeopts(opts);
@@ -961,6 +964,8 @@ vfs_domount_update(
int error, export_error, i, len;
uint64_t flag;
struct o2export_args o2export;
+ char *endptr;
+ int gotexp;
=20
ASSERT_VOP_ELOCKED(vp, __func__);
KASSERT((fsflags & MNT_UPDATE) !=3D 0, ("MNT_UPDATE should be here"));
@@ -1033,36 +1038,117 @@ vfs_domount_update(
=20
export_error =3D 0;
/* Process the export option. */
- if (error =3D=3D 0 && vfs_getopt(mp->mnt_optnew, "export", &bufp,
- &len) =3D=3D 0) {
- /* Assume that there is only 1 ABI for each length. */
- switch (len) {
- case (sizeof(struct oexport_args)):
- case (sizeof(struct o2export_args)):
- memset(&export, 0, sizeof(export));
- memset(&o2export, 0, sizeof(o2export));
- memcpy(&o2export, bufp, len);
- export.ex_flags =3D (u_int)o2export.ex_flags;
- export.ex_root =3D o2export.ex_root;
- export.ex_anon =3D o2export.ex_anon;
- export.ex_addr =3D o2export.ex_addr;
- export.ex_addrlen =3D o2export.ex_addrlen;
- export.ex_mask =3D o2export.ex_mask;
- export.ex_masklen =3D o2export.ex_masklen;
- export.ex_indexfile =3D o2export.ex_indexfile;
- export.ex_numsecflavors =3D o2export.ex_numsecflavors;
- for (i =3D 0; i < MAXSECFLAVORS; i++)
- export.ex_secflavors[i] =3D
- o2export.ex_secflavors[i];
- export_error =3D vfs_export(mp, &export);
- break;
- case (sizeof(export)):
- bcopy(bufp, &export, len);
- export_error =3D vfs_export(mp, &export);
- break;
- default:
- export_error =3D EINVAL;
- break;
+ if (error =3D=3D 0) {
+ gotexp =3D 0;
+ memset(&export, 0, sizeof(export));
+ if (vfs_getopt(mp->mnt_optnew, "export.exflags", &bufp,
+ &len) =3D=3D 0) {
+ gotexp =3D 1;
+ export.ex_flags =3D strtouq(bufp, &endptr, 0);
+ if (endptr =3D=3D bufp)
+ export_error =3D EINVAL;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.root", &bufp,
+ &len) =3D=3D 0) {
+ gotexp =3D 1;
+ export.ex_root =3D strtouq(bufp, &endptr, 0);
+ if (endptr =3D=3D bufp)
+ export_error =3D EINVAL;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.anonuid", &bufp,
+ &len) =3D=3D 0) {
+ gotexp =3D 1;
+ export.ex_anon.cr_uid =3D strtouq(bufp, &endptr, 0);
+ if (endptr !=3D bufp)
+ export.ex_anon.cr_version =3D XUCRED_VERSION;
+ else
+ export_error =3D EINVAL;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.anongroups", &bufp,
+ &len) =3D=3D 0) {
+ gotexp =3D 1;
+ export.ex_anon.cr_ngroups =3D len / sizeof(gid_t);
+ if (export.ex_anon.cr_ngroups > XU_NGROUPS) {
+ export.ex_suppgroups =3D mallocarray(
+ sizeof(gid_t),
+ export.ex_anon.cr_ngroups, M_TEMP,
+ M_WAITOK);
+ memcpy(export.ex_suppgroups, bufp, len);
+ } else
+ memcpy(export.ex_anon.cr_groups, bufp,
+ len);
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.addr", &bufp,
+ &len) =3D=3D 0) {
+ gotexp =3D 1;
+ export.ex_addr =3D malloc(len, M_TEMP, M_WAITOK);
+ memcpy(export.ex_addr, bufp, len);
+ export.ex_addrlen =3D len;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.mask", &bufp,
+ &len) =3D=3D 0) {
+ gotexp =3D 1;
+ export.ex_mask =3D malloc(len, M_TEMP, M_WAITOK);
+ memcpy(export.ex_mask, bufp, len);
+ export.ex_masklen =3D len;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.indexfile", &bufp,
+ &len) =3D=3D 0) {
+ gotexp =3D 1;
+ export.ex_indexfile =3D malloc(len + 1, M_TEMP,
+ M_WAITOK);
+ memcpy(export.ex_indexfile, bufp, len);
+ export.ex_indexfile[len] =3D '\0';
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.secflavors", &bufp,
+ &len) =3D=3D 0) {
+ gotexp =3D 1;
+ export.ex_numsecflavors =3D len / sizeof(uint32_t);
+ if (export.ex_numsecflavors <=3D MAXSECFLAVORS)
+ memcpy(export.ex_secflavors, bufp, len);
+ else
+ export_error =3D EINVAL;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.fsid", &bufp,
+ &len) =3D=3D 0) {
+ gotexp =3D 1;
+ export.ex_fsid =3D strtouq(bufp, &endptr, 0);
+ if (endptr =3D=3D bufp)
+ export_error =3D EINVAL;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export", &bufp, &len) =3D=3D 0) {
+ /* Assume that there is only 1 ABI for each length. */
+ switch (len) {
+ case (sizeof(struct oexport_args)):
+ case (sizeof(struct o2export_args)):
+ memset(&export, 0, sizeof(export));
+ memset(&o2export, 0, sizeof(o2export));
+ memcpy(&o2export, bufp, len);
+ export.ex_flags =3D (u_int)o2export.ex_flags;
+ export.ex_root =3D o2export.ex_root;
+ export.ex_anon =3D o2export.ex_anon;
+ export.ex_addr =3D o2export.ex_addr;
+ export.ex_addrlen =3D o2export.ex_addrlen;
+ export.ex_mask =3D o2export.ex_mask;
+ export.ex_masklen =3D o2export.ex_masklen;
+ export.ex_indexfile =3D o2export.ex_indexfile;
+ export.ex_numsecflavors =3D o2export.ex_numsecflavors;
+ for (i =3D 0; i < MAXSECFLAVORS; i++)
+ export.ex_secflavors[i] =3D
+ o2export.ex_secflavors[i];
+ export_error =3D vfs_export(mp, &export);
+ break;
+ default:
+ export_error =3D EINVAL;
+ break;
+ }
+ } else if (gotexp !=3D 0) {
+ if (export_error =3D=3D 0)
+ export_error =3D vfs_export(mp, &export);
+ free(export.ex_addr, M_TEMP);
+ free(export.ex_mask, M_TEMP);
+ free(export.ex_indexfile, M_TEMP);
+ free(export.ex_suppgroups, M_TEMP);
}
}
=20
So, what to people think about this? rick
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YTOPR0101MB11626B32F73B520FBDA3C633DDFA0>