From owner-freebsd-current@freebsd.org Sat Oct 20 01:17:40 2018 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4A2C5FEEA23 for ; Sat, 20 Oct 2018 01:17:40 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from CAN01-TO1-obe.outbound.protection.outlook.com (mail-eopbgr670065.outbound.protection.outlook.com [40.107.67.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.protection.outlook.com", Issuer "GlobalSign Organization Validation CA - SHA256 - G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id C728C76543; Sat, 20 Oct 2018 01:17:39 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from YTOPR0101MB1162.CANPRD01.PROD.OUTLOOK.COM (52.132.50.155) by YTOPR0101MB2027.CANPRD01.PROD.OUTLOOK.COM (52.132.49.12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1250.20; Sat, 20 Oct 2018 01:17:37 +0000 Received: from YTOPR0101MB1162.CANPRD01.PROD.OUTLOOK.COM ([fe80::b09c:c689:9517:ea2b]) by YTOPR0101MB1162.CANPRD01.PROD.OUTLOOK.COM ([fe80::b09c:c689:9517:ea2b%4]) with mapi id 15.20.1250.028; Sat, 20 Oct 2018 01:17:37 +0000 From: Rick Macklem To: Brooks Davis , FreeBSD Current , Josh Paetzel Subject: Re: which way to update export_args structure? Thread-Topic: which way to update export_args structure? Thread-Index: AQHUWq0WJEV5v0B9NEODb8Fv+fAB/qUNrJCAgAZGIdGAAanmAIAR0AV4 Date: Sat, 20 Oct 2018 01:17:37 +0000 Message-ID: References: <20181003155133.GA57729@spindle.one-eyed-alien.net> , <20181008170428.GB9766@spindle.one-eyed-alien.net> In-Reply-To: <20181008170428.GB9766@spindle.one-eyed-alien.net> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=rmacklem@uoguelph.ca; x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; YTOPR0101MB2027; 6:hvoky0Ldkv5HMrNUPNYQgtGcYhtFQoWjwOomNNKGD/Oz6hRkjhp1SCdHvbP5y3Q6syvc8+/Hmp5N84iaPRYhbj/bQwNcOJ8SVJYWpkbi/ERNlH0ZEoqm4JEPa6nWQnmdckpyM/xbEMWxAufs5VX2MhngEGTsmJ7zmlEUrctjnSUoS6Wt0FHpuR0+9Cqa2aY2i1SStNAsp19m16qDFCsHCzKfwlKYWnoLWlPmZ3yIPWrhNd1HlEozUcZLoadlD6l9cxDubQK8CBcwGsPG1t4fwSTVFfyRDzu5/ttsGeGT0Yvx7UosAwq7Ws0R/T1O6uS64xXXV5qSYagdqGNb9cnIr2FbWYsrtdtQoQJY/CTDS/SlI1WEBF6QUCTsx4lcCGax/Ccm/0oVZBFsoQaNb0ksU8hhEKC5M+x3Flc5IdUU7helnm50szS3pI9eaJJAue4e4YjW21gXO8U3G+zfffUDkQ==; 5:ZKIIeNrAJJhBq0hnXT9oBvSpeEIZMcA2nE3YP5ShzdLMxDwRyjAr7HLboxNQPABzSI8UvDFSViw1FkoRMXtvZw2YtALZZvJ/RBI5GJIYIXQc34x9k0d6jslghjqc96garXCv1ncYPWtDdSEN/RXsphGvEv3kCF3rhlkizo+bA3Y=; 7:mSHSGs6k0tgoM2HOkT+Mue7//YHbGr9qkWrcuRRIpORRU8ifxrKIIBnfM59QJyfCctPO5S6aLEew2rtGg64RdYGLB7IkxlSoC1jg8e1+RHwNGSTdN8rNcRVu6cMg3LgwJXtnFQSre5XWYpUrCZyo1GhO6BbZF8e2ta/21YI09SmNHnOoLP7AuEKU9nfbHjhNzax50uCr1BOwQwOKIw7/HscGhczaWzirMXdGAVFUkNkiVZMeJO3qhqJfqueivE0A x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: e5f5c11b-8045-40d9-b899-08d63629d2b4 x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(5600074)(711020)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020); SRVR:YTOPR0101MB2027; x-ms-traffictypediagnostic: YTOPR0101MB2027: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040522)(2401047)(5005006)(8121501046)(3002001)(93006095)(93001095)(3231355)(944501410)(52105095)(10201501046)(148016)(149066)(150057)(6041310)(20161123558120)(20161123564045)(20161123560045)(201703131423095)(201702281529075)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(201708071742011)(7699051)(76991095); SRVR:YTOPR0101MB2027; BCL:0; PCL:0; RULEID:; SRVR:YTOPR0101MB2027; x-forefront-prvs: 0831C25939 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(136003)(396003)(346002)(366004)(376002)(39860400002)(189003)(199004)(51444003)(9686003)(25786009)(74482002)(476003)(229853002)(46003)(7696005)(2900100001)(486006)(99286004)(76176011)(305945005)(74316002)(15650500001)(186003)(6506007)(14454004)(6246003)(86362001)(106356001)(5250100002)(105586002)(71190400001)(102836004)(71200400001)(53936002)(478600001)(5660300001)(256004)(68736007)(14444005)(33656002)(110136005)(8936002)(81156014)(55016002)(6436002)(11346002)(81166006)(8676002)(97736004)(2906002)(316002)(93886005)(786003)(446003)(43043002); DIR:OUT; SFP:1101; SCL:1; SRVR:YTOPR0101MB2027; H:YTOPR0101MB1162.CANPRD01.PROD.OUTLOOK.COM; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: uoguelph.ca does not designate permitted sender hosts) x-microsoft-antispam-message-info: j7dKl3s2SzdlYlrC2+pJJYkQzFrOJ/i9BYixs54jpivVN/pP+sUh8ZU647Qv61kvxy0ygLZiJ39xg/H9/ynKgoopdaalG5pGnYdwOsoqN/cKmlY7VZlo6H4M+WXESYowkGi/7P3Ebynme/U41V4pq7hqy50PobhJYyWBV+007VQMi0BWe68mU/NoBpE2/timJtT0ZgcGfbEpL1iqhRZknGywOYJSDWE2H8bsSsJNaHIaAxHXRIzdEMtkGwsJKP5UTFY+gZYYD9pgH9tK4Eb7+FOLvfPYNTuXWYHB5LisE+xl0zU4hP3ag5G7N2H/P5cJvfijbiQ7sMk8KEGhib/bOt3l2j1Vb+SrXia6IlDfliw= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: uoguelph.ca X-MS-Exchange-CrossTenant-Network-Message-Id: e5f5c11b-8045-40d9-b899-08d63629d2b4 X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Oct 2018 01:17:37.5184 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: be62a12b-2cad-49a1-a5fa-85f4f3156a7d X-MS-Exchange-Transport-CrossTenantHeadersStamped: YTOPR0101MB2027 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 20 Oct 2018 01:17:40 -0000 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