From owner-svn-src-head@freebsd.org Sun Mar 12 18:04:25 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 1F9D9D0950F for ; Sun, 12 Mar 2017 18:04:25 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from pmta2.delivery6.ore.mailhop.org (pmta2.delivery6.ore.mailhop.org [54.200.129.228]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id EDB1910E4 for ; Sun, 12 Mar 2017 18:04:24 +0000 (UTC) (envelope-from ian@freebsd.org) X-MHO-User: 363430f4-074e-11e7-b3c2-c9f38144898e X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 73.78.92.27 X-Mail-Handler: DuoCircle Outbound SMTP Received: from ilsoft.org (unknown [73.78.92.27]) by outbound2.ore.mailhop.org (Halon) with ESMTPSA id 363430f4-074e-11e7-b3c2-c9f38144898e; Sun, 12 Mar 2017 18:03:36 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id v2CI4GW9001630; Sun, 12 Mar 2017 12:04:16 -0600 (MDT) (envelope-from ian@freebsd.org) Message-ID: <1489341856.40576.88.camel@freebsd.org> Subject: Re: svn commit: r314780 - head/lib/libpam/modules/pam_exec From: Ian Lepore To: Pedro Giffuni , Lawrence Stewart , src-committers@freebsd.org, Dag-Erling =?ISO-8859-1?Q?Sm=F8rgrav?= , svn-src-head@freebsd.org Date: Sun, 12 Mar 2017 12:04:16 -0600 In-Reply-To: <1839903b-fb05-bf3f-17bb-697afca9ecb7@FreeBSD.org> References: <201703061545.v26FjkNI027057@repo.freebsd.org> <739617a4-3eed-28d1-73e4-86d25d6d5fed@freebsd.org> <1839903b-fb05-bf3f-17bb-697afca9ecb7@FreeBSD.org> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.18.5.1 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 12 Mar 2017 18:04:25 -0000 On Sun, 2017-03-12 at 12:30 -0500, Pedro Giffuni wrote: > > On 3/12/2017 12:14 PM, Lawrence Stewart wrote: > > > > Hi Pedro, > > > > On 07/03/2017 02:45, Pedro F. Giffuni wrote: > > > > > > Author: pfg > > > Date: Mon Mar  6 15:45:46 2017 > > > New Revision: 314780 > > > URL: https://svnweb.freebsd.org/changeset/base/314780 > > > > > > Log: > > >    libpam: extra bounds checking through reallocarray(3). > > >     > > >    Reviewed by: des > > >    MFC after: 1 week > > > > > > Modified: > > >    head/lib/libpam/modules/pam_exec/pam_exec.c > > > > > > Modified: head/lib/libpam/modules/pam_exec/pam_exec.c > > > ================================================================= > > > ============= > > > --- head/lib/libpam/modules/pam_exec/pam_exec.c Mon Mar  6 > > > 15:42:03 2017 (r314779) > > > +++ head/lib/libpam/modules/pam_exec/pam_exec.c Mon Mar  6 > > > 15:45:46 2017 (r314780) > > > @@ -138,7 +138,7 @@ _pam_exec(pam_handle_t *pamh __unused, > > >    nitems = sizeof(env_items) / sizeof(*env_items); > > >    /* Count PAM return values put in the environment. */ > > >    nitems_rv = options->return_prog_exit_status ? > > > PAM_RV_COUNT : 0; > > > - tmp = realloc(envlist, (envlen + nitems + 1 + nitems_rv > > > + 1) * > > > + tmp = reallocarray(envlist, envlen + nitems + 1 + > > > nitems_rv + 1, > > >        sizeof(*envlist)); > > >    if (tmp == NULL) { > > >    openpam_free_envlist(envlist); > > > > > This commit breaks pam_exec for me... without this change I see the > > expected PAM_* environment variables from my execed script, but > > with > > this change I no longer see any of them. > Thanks for the report. > > It seems strange this can cause any failure. Perhaps there is a > latent  > overflow here and we have been living with it? I will revert while it > is  > investigated. > > BTW, the "nitems" variable may conflict with nitems() in sys/param.h. > A quirk of C that's often forgotten is that a function-like macro is only expanded as a macro if the token following the macro name is an open paren.  So nitems() is a macro invokation and nitems = 0; is just a variable. I'm not arguing against the replacement of variables named nitems, I actually think that should have been done as part of importing the function-like definition of nitems from netbsd. -- Ian