which way to update export_args structure?
Rick Macklem
rmacklem at uoguelph.ca
Sat Oct 20 01:17:40 UTC 2018
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 in 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 in 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 kernel?)
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;
opts = malloc(sizeof(struct vfsoptlist), M_MOUNT, M_WAITOK);
TAILQ_INIT(opts);
@@ -325,7 +326,7 @@ vfs_buildopts(struct uio *auio, struct v
}
if (optlen != 0) {
opt->len = optlen;
- opt->value = malloc(optlen, M_MOUNT, M_WAITOK);
+ opt->value = malloc(optlen + 1, M_MOUNT, M_WAITOK);
if (auio->uio_segflg == 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 = (char *)opt->value;
+ cp[optlen] = '\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;
ASSERT_VOP_ELOCKED(vp, __func__);
KASSERT((fsflags & MNT_UPDATE) != 0, ("MNT_UPDATE should be here"));
@@ -1033,36 +1038,117 @@ vfs_domount_update(
export_error = 0;
/* Process the export option. */
- if (error == 0 && vfs_getopt(mp->mnt_optnew, "export", &bufp,
- &len) == 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 = (u_int)o2export.ex_flags;
- export.ex_root = o2export.ex_root;
- export.ex_anon = o2export.ex_anon;
- export.ex_addr = o2export.ex_addr;
- export.ex_addrlen = o2export.ex_addrlen;
- export.ex_mask = o2export.ex_mask;
- export.ex_masklen = o2export.ex_masklen;
- export.ex_indexfile = o2export.ex_indexfile;
- export.ex_numsecflavors = o2export.ex_numsecflavors;
- for (i = 0; i < MAXSECFLAVORS; i++)
- export.ex_secflavors[i] =
- o2export.ex_secflavors[i];
- export_error = vfs_export(mp, &export);
- break;
- case (sizeof(export)):
- bcopy(bufp, &export, len);
- export_error = vfs_export(mp, &export);
- break;
- default:
- export_error = EINVAL;
- break;
+ if (error == 0) {
+ gotexp = 0;
+ memset(&export, 0, sizeof(export));
+ if (vfs_getopt(mp->mnt_optnew, "export.exflags", &bufp,
+ &len) == 0) {
+ gotexp = 1;
+ export.ex_flags = strtouq(bufp, &endptr, 0);
+ if (endptr == bufp)
+ export_error = EINVAL;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.root", &bufp,
+ &len) == 0) {
+ gotexp = 1;
+ export.ex_root = strtouq(bufp, &endptr, 0);
+ if (endptr == bufp)
+ export_error = EINVAL;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.anonuid", &bufp,
+ &len) == 0) {
+ gotexp = 1;
+ export.ex_anon.cr_uid = strtouq(bufp, &endptr, 0);
+ if (endptr != bufp)
+ export.ex_anon.cr_version = XUCRED_VERSION;
+ else
+ export_error = EINVAL;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.anongroups", &bufp,
+ &len) == 0) {
+ gotexp = 1;
+ export.ex_anon.cr_ngroups = len / sizeof(gid_t);
+ if (export.ex_anon.cr_ngroups > XU_NGROUPS) {
+ export.ex_suppgroups = 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) == 0) {
+ gotexp = 1;
+ export.ex_addr = malloc(len, M_TEMP, M_WAITOK);
+ memcpy(export.ex_addr, bufp, len);
+ export.ex_addrlen = len;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.mask", &bufp,
+ &len) == 0) {
+ gotexp = 1;
+ export.ex_mask = malloc(len, M_TEMP, M_WAITOK);
+ memcpy(export.ex_mask, bufp, len);
+ export.ex_masklen = len;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.indexfile", &bufp,
+ &len) == 0) {
+ gotexp = 1;
+ export.ex_indexfile = malloc(len + 1, M_TEMP,
+ M_WAITOK);
+ memcpy(export.ex_indexfile, bufp, len);
+ export.ex_indexfile[len] = '\0';
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.secflavors", &bufp,
+ &len) == 0) {
+ gotexp = 1;
+ export.ex_numsecflavors = len / sizeof(uint32_t);
+ if (export.ex_numsecflavors <= MAXSECFLAVORS)
+ memcpy(export.ex_secflavors, bufp, len);
+ else
+ export_error = EINVAL;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export.fsid", &bufp,
+ &len) == 0) {
+ gotexp = 1;
+ export.ex_fsid = strtouq(bufp, &endptr, 0);
+ if (endptr == bufp)
+ export_error = EINVAL;
+ }
+ if (vfs_getopt(mp->mnt_optnew, "export", &bufp, &len) == 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 = (u_int)o2export.ex_flags;
+ export.ex_root = o2export.ex_root;
+ export.ex_anon = o2export.ex_anon;
+ export.ex_addr = o2export.ex_addr;
+ export.ex_addrlen = o2export.ex_addrlen;
+ export.ex_mask = o2export.ex_mask;
+ export.ex_masklen = o2export.ex_masklen;
+ export.ex_indexfile = o2export.ex_indexfile;
+ export.ex_numsecflavors = o2export.ex_numsecflavors;
+ for (i = 0; i < MAXSECFLAVORS; i++)
+ export.ex_secflavors[i] =
+ o2export.ex_secflavors[i];
+ export_error = vfs_export(mp, &export);
+ break;
+ default:
+ export_error = EINVAL;
+ break;
+ }
+ } else if (gotexp != 0) {
+ if (export_error == 0)
+ export_error = 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);
}
}
So, what to people think about this? rick
More information about the freebsd-current
mailing list