From owner-freebsd-bugs@FreeBSD.ORG Thu Dec 25 10:20:20 2003 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id BF67416A4CE for ; Thu, 25 Dec 2003 10:20:20 -0800 (PST) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 16D5B43D2D for ; Thu, 25 Dec 2003 10:20:19 -0800 (PST) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) hBPIKIFR096236 for ; Thu, 25 Dec 2003 10:20:18 -0800 (PST) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.10/8.12.10/Submit) id hBPIKIvq096235; Thu, 25 Dec 2003 10:20:18 -0800 (PST) (envelope-from gnats) Date: Thu, 25 Dec 2003 10:20:18 -0800 (PST) Message-Id: <200312251820.hBPIKIvq096235@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Friedemann Becker Subject: Re: bin/2442: setusershell()/endusershell() missing X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Friedemann Becker List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Dec 2003 18:20:21 -0000 The following reply was made to PR bin/2442; it has been noted by GNATS. From: Friedemann Becker To: freebsd-gnats-submit@FreeBSD.org, charnier@xp11.frmug.org Cc: Subject: Re: bin/2442: setusershell()/endusershell() missing Date: Thu, 25 Dec 2003 19:15:24 +0100 This is was i found about the xxxusershell issue: in src/lib/libc/gen/getusershell.c a stringlist is used to store the /etc/shells entries. in setusershell(), or getusershell() when called for the first time, the memory for the string list is allocated (see stringlist.c). subsequent calls to getusershell() just return the next entry in the list, one after one. endusershell() deallocates the memory. now, in the patches, XXXusershell() is only used once in each case, so it doesn't matter, if setusershell() is called or not. this only becomes a problem, if eventually someone decides to use these functions more than once per process. a missing call to endusershell() is not a problem, the memory is free'd at the next call to setusershell() or process termination. someone should decide, what to do here. i have reviewed each of the patches, in case someone wants to apply them. ftpd.c: the patch for ftpd still applies and is correct --- /usr/src/libexec/ftpd/ftpd.c Thu Dec 11 19:07:26 2003 +++ /home/lucius/usr.src/libexec/ftpd/ftpd.c Thu Dec 25 18:02:57 2003 @@ -1015,6 +1015,7 @@ if ((pw = sgetpwnam(name))) { if ((shell = pw->pw_shell) == NULL || *shell == 0) shell = _PATH_BSHELL; + setusershell(); while ((cp = getusershell()) != NULL) if (strcmp(cp, shell) == 0) break; user.c: this one still applies, is correct, but different path note: the result has not to be strdup'ed, only the pointer is tested ==NULL, the pointer-address isn't affected by free (called in endusershell()). --- /usr/src/usr.sbin/sysinstall/user.c Thu Jul 5 11:51:09 2001 +++ /home/lucius/usr.src/usr.sbin/sysinstall/user.c Thu Dec 25 18:44:36 2003 @@ -426,6 +426,7 @@ return 0; } if (strlen(shell) > 0) { + setusershell(); while((cp = getusershell()) != NULL) if (strcmp(cp, shell) == 0) break; util.c: this patch had to be corrected as described in the audit-trail. now working --- /usr/src/usr.bin/chpass/util.c Sun Jun 30 07:21:15 2002 +++ /home/lucius/usr.src/usr.bin/chpass/util.c Thu Dec 25 18:14:19 2003 @@ -142,15 +142,21 @@ char * ok_shell(char *name) { - char *p, *sh; + char *p, *sh, *ret; setusershell(); while ((sh = getusershell())) { - if (!strcmp(name, sh)) + if (!strcmp(name, sh)) { + endusershell(); return (name); + } /* allow just shell name, but use "real" path */ - if ((p = strrchr(sh, '/')) && strcmp(name, p + 1) == 0) - return (sh); + if ((p = strrchr(sh, '/')) && strcmp(name, p + 1) == 0) { + ret = strdup(sh); + endusershell(); + return (ret); + } } + endusershell(); return (NULL); } su.c: this patch is obsolete - {set,get,end}usershell is called correctly (at least in -current) pw_scan: here the curly braces were patched away (the 'for' line), this was corrected below --- /usr/src/lib/libc/gen/pw_scan.c Fri Oct 11 13:35:30 2002 +++ /home/lucius/usr.src/lib/libc/gen/pw_scan.c Thu Dec 25 18:29:28 2003 @@ -180,7 +180,7 @@ goto fmt; p = pw->pw_shell; - if (root && *p) /* empty == /bin/sh */ + if (root && *p) { /* empty == /bin/sh */ for (setusershell();;) { if (!(sh = getusershell())) { if (flags & _PWSCAN_WARN) @@ -190,6 +190,8 @@ if (!strcmp(p, sh)) break; } + endusershell(); + } if (p[0]) pw->pw_fields |= _PWF_SHELL;