Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 10 Oct 2020 00:01:40 +0000 (UTC)
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r366595 - head/usr.sbin/mountd
Message-ID:  <202010100001.09A01ew2072529@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: rmacklem
Date: Sat Oct 10 00:01:40 2020
New Revision: 366595
URL: https://svnweb.freebsd.org/changeset/base/366595

Log:
  Modify mountd.c so that it does not always malloc 4K for the map credentials.
  
  r362163 upgraded mountd so that it could handle MAX_NGROUPS
  groups for the anonymous user credentials (the ones provided by
  -maproot and -mapall exports options).
  The problem is that this resulted in every export structure growing by
  about 4Kbytes, because the cr_groups field went from 16->MAX_NGROUPS.
  
  This patch fixes this by only including a small 32 element cr_groups in the
  structure and then malloc()'ng cr_groups when a larger one is needed.
  The value of SMALLNGROUPS is arbitrarily set to 32, assuming most users
  used by -maproot or -mapall will be in <= 32 groups.
  
  Reviewed by:	kib, freqlabs
  Differential Revision:	https://reviews.freebsd.org/D26521

Modified:
  head/usr.sbin/mountd/mountd.c

Modified: head/usr.sbin/mountd/mountd.c
==============================================================================
--- head/usr.sbin/mountd/mountd.c	Fri Oct  9 23:49:42 2020	(r366594)
+++ head/usr.sbin/mountd/mountd.c	Sat Oct 10 00:01:40 2020	(r366595)
@@ -115,11 +115,15 @@ struct dirlist {
 
 /*
  * maproot/mapall credentials.
+ * cr_smallgrps can be used for a group list up to SMALLNGROUPS in size.
+ * Larger group lists are malloc'd/free'd.
  */
+#define	SMALLNGROUPS	32
 struct expcred {
 	uid_t		cr_uid;
 	int		cr_ngroups;
-	gid_t		cr_groups[NGROUPS_MAX + 1];
+	gid_t		cr_smallgrps[SMALLNGROUPS];
+	gid_t		*cr_groups;
 };
 
 struct exportlist {
@@ -1514,6 +1518,7 @@ get_exportlist_one(int passno)
 	uint64_t exflags;
 
 	v4root_phase = 0;
+	anon.cr_groups = NULL;
 	dirhead = (struct dirlist *)NULL;
 	while (get_line()) {
 		if (debug)
@@ -1527,6 +1532,7 @@ get_exportlist_one(int passno)
 		 * Set defaults.
 		 */
 		has_host = FALSE;
+		anon.cr_groups = anon.cr_smallgrps;
 		anon.cr_uid = UID_NOBODY;
 		anon.cr_ngroups = 1;
 		anon.cr_groups[0] = GID_NOGROUP;
@@ -1822,6 +1828,10 @@ nextline:
 			free_dir(dirhead);
 			dirhead = (struct dirlist *)NULL;
 		}
+		if (anon.cr_groups != anon.cr_smallgrps) {
+			free(anon.cr_groups);
+			anon.cr_groups = NULL;
+		}
 	}
 }
 
@@ -2905,6 +2915,8 @@ free_exp(struct exportlist *ep)
 		grp = grp->gr_next;
 		free_grp(tgrp);
 	}
+	if (ep->ex_defanon.cr_groups != ep->ex_defanon.cr_smallgrps)
+		free(ep->ex_defanon.cr_groups);
 	free((caddr_t)ep);
 }
 
@@ -3457,14 +3469,17 @@ static void
 parsecred(char *namelist, struct expcred *cr)
 {
 	char *name;
-	int cnt;
+	int inpos;
 	char *names;
 	struct passwd *pw;
 	struct group *gr;
+	gid_t groups[NGROUPS_MAX + 1];
+	int ngroups;
 
 	/*
 	 * Set up the unprivileged user.
 	 */
+	cr->cr_groups = cr->cr_smallgrps;
 	cr->cr_uid = UID_NOBODY;
 	cr->cr_groups[0] = GID_NOGROUP;
 	cr->cr_ngroups = 1;
@@ -3487,24 +3502,28 @@ parsecred(char *namelist, struct expcred *cr)
 			return;
 		}
 		cr->cr_uid = pw->pw_uid;
-		cr->cr_ngroups = NGROUPS_MAX + 1;
-		if (getgrouplist(pw->pw_name, pw->pw_gid, cr->cr_groups,
-		    &cr->cr_ngroups)) {
+		ngroups = NGROUPS_MAX + 1;
+		if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups)) {
 			syslog(LOG_ERR, "too many groups");
-			cr->cr_ngroups = NGROUPS_MAX + 1;
+			ngroups = NGROUPS_MAX + 1;
 		}
 
 		/*
 		 * Compress out duplicate.
 		 */
-		if (cr->cr_ngroups > 1 && cr->cr_groups[0] ==
-		    cr->cr_groups[1]) {
-			for (cnt = 2; cnt < cr->cr_ngroups; cnt++)
-				cr->cr_groups[cnt - 1] = cr->cr_groups[cnt];
-			cr->cr_ngroups--;
-		}
-		if (cr->cr_ngroups > NGROUPS_MAX)
-			cr->cr_ngroups = NGROUPS_MAX;
+		if (ngroups > 1 && groups[0] == groups[1]) {
+			ngroups--;
+			inpos = 2;
+		} else
+			inpos = 1;
+		if (ngroups > NGROUPS_MAX)
+			ngroups = NGROUPS_MAX;
+		if (ngroups > SMALLNGROUPS)
+			cr->cr_groups = malloc(ngroups * sizeof(gid_t));
+		cr->cr_ngroups = ngroups;
+		cr->cr_groups[0] = groups[0];
+		memcpy(&cr->cr_groups[1], &groups[inpos], (ngroups - 1) *
+		    sizeof(gid_t));
 		return;
 	}
 	/*
@@ -3523,17 +3542,20 @@ parsecred(char *namelist, struct expcred *cr)
 	while (names != NULL && *names != '\0' && cr->cr_ngroups < NGROUPS_MAX) {
 		name = strsep_quote(&names, ":");
 		if (isdigit(*name) || *name == '-') {
-			cr->cr_groups[cr->cr_ngroups++] = atoi(name);
+			groups[cr->cr_ngroups++] = atoi(name);
 		} else {
 			if ((gr = getgrnam(name)) == NULL) {
 				syslog(LOG_ERR, "unknown group: %s", name);
 				continue;
 			}
-			cr->cr_groups[cr->cr_ngroups++] = gr->gr_gid;
+			groups[cr->cr_ngroups++] = gr->gr_gid;
 		}
 	}
 	if (names != NULL && *names != '\0' && cr->cr_ngroups == NGROUPS_MAX)
 		syslog(LOG_ERR, "too many groups");
+	if (cr->cr_ngroups > SMALLNGROUPS)
+		cr->cr_groups = malloc(cr->cr_ngroups * sizeof(gid_t));
+	memcpy(cr->cr_groups, groups, cr->cr_ngroups * sizeof(gid_t));
 }
 
 #define	STRSIZ	(MNTNAMLEN+MNTPATHLEN+50)
@@ -3642,6 +3664,8 @@ free_grp(struct grouplist *grp)
 		if (grp->gr_ptr.gt_net.nt_name)
 			free(grp->gr_ptr.gt_net.nt_name);
 	}
+	if (grp->gr_anon.cr_groups != grp->gr_anon.cr_smallgrps)
+		free(grp->gr_anon.cr_groups);
 	free((caddr_t)grp);
 }
 
@@ -3860,6 +3884,10 @@ cp_cred(struct expcred *outcr, struct expcred *incr)
 
 	outcr->cr_uid = incr->cr_uid;
 	outcr->cr_ngroups = incr->cr_ngroups;
+	if (outcr->cr_ngroups > SMALLNGROUPS)
+		outcr->cr_groups = malloc(outcr->cr_ngroups * sizeof(gid_t));
+	else
+		outcr->cr_groups = outcr->cr_smallgrps;
 	memcpy(outcr->cr_groups, incr->cr_groups, incr->cr_ngroups *
 	    sizeof(gid_t));
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202010100001.09A01ew2072529>