From owner-freebsd-current@FreeBSD.ORG Wed Feb 20 01:52:56 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id BC3C83C3 for ; Wed, 20 Feb 2013 01:52:56 +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 74E2A8B7 for ; Wed, 20 Feb 2013 01:52:56 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AqEEAGMrJFGDaFvO/2dsb2JhbABFhkm6AYEkc4IfAQEBBAEBASAEJyALBRYOCgICDRkCKQEJJgYIBwQBGgIEh3EMrWyCQJAggSOMJQUGgQc0B4ItgRMDiGaLDYI4gR2PO4MlT30IFx4 X-IronPort-AV: E=Sophos;i="4.84,698,1355115600"; d="scan'208";a="17368554" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-jnhn.mail.uoguelph.ca with ESMTP; 19 Feb 2013 20:52:49 -0500 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id DCA7CB3F17; Tue, 19 Feb 2013 20:52:49 -0500 (EST) Date: Tue, 19 Feb 2013 20:52:49 -0500 (EST) From: Rick Macklem To: Andrey Simonenko Message-ID: <747768617.3137250.1361325169865.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20130219102744.GA2426@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.203] 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 01:52:56 -0000 Andrey Simonenko wrote: > On Tue, Feb 19, 2013 at 05:35:50PM +0800, Elias Martenson wrote: > > On 19 February 2013 17:31, Andrey Simonenko > > wrote: > > > > It can require bigger buffer, since root can get the pw_password > > field > > > in the struct passwd{}. > > > > > > Since sysconf(_SC_GETPW_R_SIZE_MAX) does not work on FreeBSD, the > > > buffer > > > for getpwnam_r() call should have at least (2 * MAXLOGNAME + 2 * > > > MAXPATHLEN + > > > _PASSWORD_LEN + 1) bytes (it is unclear how much is required for > > > pw_gecos). > > > > > > This buffer can be dynamically reallocated until getpwnam_r() is > > > not > > > return ERANGE error (the following code has not been compiled and > > > verified): > > > > > > > Is this really a better solution than to aim high right away? A > > series of > > malloc() calls should certainly have much higher overhead than the > > previous > > stack-allocated solution. > > > > A better compromise would be to do the lookup in a separate > > function, that > > allocates the buffer using alloca() instead, yes? > > 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 ???) Btw, the same fix is needed in gssd.c, where it calls getpwuid_r(). { Interesting that for Elias's case, it must work for 128, although the getpwnam_r() didn't quite fit in 128. } Also, FYI, kuserok.c uses a 2048 byte buffer and doesn't check for ERANGE. rick > _______________________________________________ > 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"