From owner-freebsd-current@FreeBSD.ORG Wed Feb 20 23:29:08 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id E1FFC3F2 for ; Wed, 20 Feb 2013 23:29:08 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-jnhn.mail.uoguelph.ca (esa-jnhn.mail.uoguelph.ca [131.104.91.44]) by mx1.freebsd.org (Postfix) with ESMTP id A9C59235 for ; Wed, 20 Feb 2013 23:29:08 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AqEEAHVbJVGDaFvO/2dsb2JhbABFhkm6GoEac4IfAQEBAwEBAQEgBCcgCwUWDgoCAg0ZAikBCSYGCAcEARwEh2sGDK4Akj2BI4wlBQaBBzQHgi2BEwOIZosOgjiBHY8+gyVPfQgXHg X-IronPort-AV: E=Sophos;i="4.84,705,1355115600"; d="scan'208";a="17540792" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-jnhn.mail.uoguelph.ca with ESMTP; 20 Feb 2013 18:29:07 -0500 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 23641B3EB3; Wed, 20 Feb 2013 18:29:07 -0500 (EST) Date: Wed, 20 Feb 2013 18:29:07 -0500 (EST) From: Rick Macklem To: Andrey Simonenko Message-ID: <423251068.3167476.1361402947119.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20130220122826.GA13613@pm513-1.comsys.ntu-kpi.kiev.ua> Subject: Re: Possible bug in NFSv4 with krb5p security? MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [172.17.91.202] X-Mailer: Zimbra 6.0.10_GA_2692 (ZimbraWebClient - FF3.0 (Win)/6.0.10_GA_2692) Cc: freebsd-current@freebsd.org, Elias Martenson , Benjamin Kaduk X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 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: Wed, 20 Feb 2013 23:29:08 -0000 Andrey Simonnenko wrote: > On Tue, Feb 19, 2013 at 08:52:49PM -0500, Rick Macklem wrote: > > > > > > I cannot find how to get information about maximum buffer size for > > > the getpwnam_r() function. This information should be returned by > > > sysconf(_SC_GETPW_R_SIZE_MAX), but since it does not work on > > > FreeBSD > > > it is necessary to guess its size. Original value is 128 and it > > > works > > > for somebody, 1024 works for your environment, but it can fail for > > > another environment. > > > > > > SUSv4 specifies "Storage referenced by the structure is allocated > > > from > > > the memory provided with the buffer parameter", but then tells > > > about > > > groups > > > in EXAMPLE for getpwnam_r() "Note that > > > sysconf(_SC_GETPW_R_SIZE_MAX) > > > may > > > return -1 if there is no hard limit on the size of the buffer > > > needed > > > to > > > store all the groups returned". > > > > > > malloc() can give overhead, but that function can try to call > > > getpwnam_r() > > > with buffer allocated from stack and if getpwnam_r() failed with > > > ERANGE > > > use dynamically allocated buffer. > > > > > > #define PWBUF_SIZE_INI (2 * MAXLOGNAME + 2 * MAXPATHLEN + > > > _PASSWORD_LEN + 1) > > > #define PWBUF_SIZE_INC 128 > > > > > > char bufs[2 * MAXLOGNAME + MAXPATHLEN + PASSWORD_LEN + 1 + 32]; > > > > > > error = getpwnam_r(lname, &pwd, bufs, sizeof(bufs), &pw); > > > if (pw != NULL) { > > > *uidp = pw->pw_uid; > > > return (GSS_S_COMPLETE); > > > } else if (error != ERANGE) > > > return (GSS_S_FAILURE); > > > > > > size = PWBUF_SIZE_INI; > > > for (;;) { > > > size += PWBUF_SIZE_INC; > > > buf = malloc(size); > > > if (buf == NULL) > > > return (GSS_S_FAILURE); > > > error = getpwnam_r(lname, &pwd, buf, size, &pw); > > > free(buf); > > > if (pw != NULL) { > > > *uidp = pw->pw_uid; > > > return (GSS_S_COMPLETE); > > > } else { > > > if (error == ERANGE && > > > size <= SIZE_MAX - PWBUF_SIZE_INC) > > > continue; > > > return (GSS_S_FAILURE); > > > } > > > } > > > > Just my opinion, but I think the above is a good approach. > > (ie. First trying a fairly large buffer on the stack that > > will succeed 99.99% of the time, but check for ERANGE and > > loop trying progressively larger malloc'd buffers until > > it stops reporting ERANGE.) > > > > I suspect the overheads behind getpwnam_r() are larger than > > the difference between using a buffer on the stack vs malloc, > > so I think it should use a fairly large buffer the first time. > > > > Personally, I might have coded it as a single do { } while(), > > with the first attempt in it, but that's just personal stylistic > > taste. (You can check for buf != bufs before doing a free() of it.) > > And, if you wanted to be clever, the code could use a static > > "bufsiz_hint", > > which is set to the largest size needed sofar and that is used as > > the initial malloc size. That way it wouldn't loop as much for a > > site with huge passwd entries. (An entire bio in the GECOS field or > > ???) > > > > Thanks for the review. > > Another variant. This is a program that can be used for verifying > correctness of the function, just change PWBUF_SIZE_* values and put > some printfs to see when buffer is reallocated. sizehint is updated > only when malloc() succeeded. > > --------------------------------- > #include > #include > > #include > > #include > #include > #include > #include > #include > #include > #include > > static int > getpwnam_r_func(const char *name, uid_t *uidp) > { > #define PWBUF_SIZE_INI (2 * MAXLOGNAME + MAXPATHLEN + _PASSWORD_LEN) > #define PWBUF_SIZE_INC 128 > > static size_t sizehint = PWBUF_SIZE_INI; > > struct passwd pwd; > struct passwd *pw; > char *buf; > size_t size; > int error; > char lname[MAXLOGNAME]; > char bufs[PWBUF_SIZE_INI]; > > strncpy(lname, name, sizeof(lname)); > > buf = bufs; > size = sizeof(bufs); > for (;;) { > error = getpwnam_r(lname, &pwd, buf, size, &pw); > if (buf != bufs) > free(buf); > if (pw != NULL) { > *uidp = pw->pw_uid; > return (GSS_S_COMPLETE); > } else if (error != ERANGE || size > SIZE_MAX - PWBUF_SIZE_INC) > return (GSS_S_FAILURE); > if (size != sizehint) > size = sizehint; > else > size += PWBUF_SIZE_INC; > buf = malloc(size); > if (buf == NULL) > return (GSS_S_FAILURE); > sizehint = size; > } > } > All looks fine to me. (Before my mailer messed with the whitespace;-) Thanks, rick ps: I will commit it in April, unless someone else does so sooner. > int > main(void) > { > const char *str[] = { "man", "root", "q", "bin", NULL }; > uid_t uid; > u_int i; > > for (i = 0; str[i] != NULL; ++i) { > printf("%-20s\t", str[i]); > if (getpwnam_r_func(str[i], &uid) == GSS_S_COMPLETE) > printf("%u\n", uid); > else > printf("not found\n"); > } > return (0); > } > --------------------------------- > _______________________________________________ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to > "freebsd-current-unsubscribe@freebsd.org"