From owner-freebsd-current@FreeBSD.ORG Mon Mar 25 14:14:26 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 125BCFD2 for ; Mon, 25 Mar 2013 14:14:26 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-annu.net.uoguelph.ca (esa-annu.mail.uoguelph.ca [131.104.91.36]) by mx1.freebsd.org (Postfix) with ESMTP id CE59F192 for ; Mon, 25 Mar 2013 14:14:25 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AqEEAGFbUFGDaFvO/2dsb2JhbABEiEW9GYIVdIIkAQEBAwEBAQEgBCcgCwUWDgoCAg0ZAikBCSYGCAcEARwEh20GDLA1kiCBI4w1EHw0B4ItgRMDlCmCPoEfj2WDJiAygQU1 X-IronPort-AV: E=Sophos;i="4.84,905,1355115600"; d="scan'208";a="20638042" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-annu.net.uoguelph.ca with ESMTP; 25 Mar 2013 10:13:54 -0400 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 9BB20B3F12; Mon, 25 Mar 2013 10:13:54 -0400 (EDT) Date: Mon, 25 Mar 2013 10:13:54 -0400 (EDT) From: Rick Macklem To: Andrey Simonenko Message-ID: <1394893012.119341.1364220834625.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20130325133424.GA1342@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.201] 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: Mon, 25 Mar 2013 14:14:26 -0000 Andrey Simonenko wrote: > On Wed, Feb 20, 2013 at 06:29:07PM -0500, Rick Macklem wrote: > > Andrey Simonnenko wrote: > > > > > > 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. > > > > > > --------------------------------- > > > 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. > > I was thinking about this approach once again and made a conclusion > that > it is wrong. Using static buffer at first and then allocate memory for > next calls can cause slowdown, depends on number of entries and used > database backend of course. The libc code for getpwnam() allocates > memory for the buffer and does not free it on exit from the function, > Not sure what you were saying by the last sentence? Using a static buffer here would make the code unsafe for multiple threads. Although the gssd is currently single threaded, that might change someday, maybe? (Using the static as a "hint" should be safe for multiple threads, since it is just a hint.) > If the above written code or any of its modification is going to be > committed to the source base (by you or by some another committer), > then I ask and require to not write/mention my name and email address > neither in source file nor in commit log message. > Ok, that's your choice. I think the code is fine, since the likelyhood of the first getpwuid_r() with the buffer on the stack failing is nearly 0, given that it is much bigger than 128. (Although I think a loop on ERANGE should be in the code, just making the buffer a lot bigger than 128 will fix the problem for 99.99999% of the cases.) The only change I was planning to the above was moving the free(buf) down until after "pwd" has been used, so it is safe for fields like pw_name, which I think will live in "buf". That way the code won't be broken if/when someone uses it for pw_name. rick > Appropriate commit log message for the above written code can be the > following: > > -------------------------------------------------------------------------- > Since FreeBSD does not support sysconf(_SC_GETPW_R_SIZE_MAX), then > allocate > a buffer of sufficient size for getpwnam_r() as it is suggested in > EXAMPLES > of SUSv4 documentation for the getpwnam_r() function. > -------------------------------------------------------------------------- > > since this documentation has similar code. > _______________________________________________ > 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"