From owner-svn-src-head@freebsd.org Sun Mar 12 17:54:16 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 6E45FD09109 for ; Sun, 12 Mar 2017 17:54:16 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from nm16-vm3.bullet.mail.ne1.yahoo.com (nm16-vm3.bullet.mail.ne1.yahoo.com [98.138.91.146]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 417591703 for ; Sun, 12 Mar 2017 17:54:16 +0000 (UTC) (envelope-from pfg@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1489341249; bh=kZUcHMati0ecJRYuB6Rl45fYGapwR2qDrvufx+IwSCw=; h=Subject:To:References:From:Date:In-Reply-To:From:Subject; b=B8A8cKil1JUBdC80xvdooJQCRzhtiCWSMOpKJR/x5G0arKRhAN6Xjk/iBMQaqXYFmX3kaJevRMVRkSES22bMoy8WVwHhtuMrsK5DVHktOoXxxu3KtTdwCdzkxrClBsbbPNyreZhxkV5zuawqROT09GAtzvXNBZlUtLSvEN+6NP5ABqTTXo/tUTNLNQhUkhFs/5ecOQ70r/NgbXm+ZuPO6obxE6q1Sgn0MCIalGbElxVrQqzkf42FHC2t73nqGwjwoI938LXEx2y2HbQHlwZ/saSJCV+nmT0qcIyCWzQc+jp+w7+ZK40wELuGnrYgZ8EoB9fhOU35w2LgxJtkW8gNZA== Received: from [98.138.101.128] by nm16.bullet.mail.ne1.yahoo.com with NNFMP; 12 Mar 2017 17:54:09 -0000 Received: from [98.138.226.132] by tm16.bullet.mail.ne1.yahoo.com with NNFMP; 12 Mar 2017 17:54:08 -0000 Received: from [127.0.0.1] by smtp219.mail.ne1.yahoo.com with NNFMP; 12 Mar 2017 17:54:08 -0000 X-Yahoo-Newman-Id: 983024.60398.bm@smtp219.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: FKZB_qAVM1kjReVCdDnASvLI5Rx_9oU4R.QRU6BvuibFLIo SlwaMiM2lEaAIgaTLZyy5VVXpgYSCza4ePL2nOQB1L5zmOgQqjown6B3Nlzg iPwjwCNzqTjIelIzrR6sJd2NYj21QkvBMzRTn5MmwqS0YyNo5spTMlNk0Jlc 4hZXuthANCeQAcNjY7F8uEw.DfgnA4QrG1HI04We.QHjd81Ya8lHIqofBpte 7JPlpqsrb4B5UgFv_eeeukKSP6FuxeACgZKK.nj.ZDDTskeUyCnfd88c5nsQ r8SQ2ICpyH61gthxqDOpXaXJsLaT4czrhNsY7.Bt2PStTNrc9ehqA1W2Yho_ 1JhUKpXKUfg7LmqsIL43kuB2f8eisbfHAkht15.tO5aTiSO7oxMtvlpuAzrc UQMLpJO67nswLOhXg1GVJ277nh5r9t79Lj1TbP_WHcjejZDt1eSF_NgcOCZz .jMJoC1qk2Tc3FDh33xMIUbKUsoAUFCdUui6dAZFzqP7qvtsJOmlLcxKIOIM WxGZj00lWK2CCTELjzuqmCG.lTtzWovgbN_DcQs3g2aN3_Ko- X-Yahoo-SMTP: xcjD0guswBAZaPPIbxpWwLcp9Unf Subject: Re: svn commit: r314780 - head/lib/libpam/modules/pam_exec To: Lawrence Stewart , src-committers@FreeBSD.org, =?UTF-8?Q?Dag-Erling_Sm=c3=b8rgrav?= , svn-src-head@FreeBSD.org References: <201703061545.v26FjkNI027057@repo.freebsd.org> <739617a4-3eed-28d1-73e4-86d25d6d5fed@freebsd.org> <1839903b-fb05-bf3f-17bb-697afca9ecb7@FreeBSD.org> From: Pedro Giffuni Message-ID: Date: Sun, 12 Mar 2017 12:57:11 -0500 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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 17:54:16 -0000 On 3/12/2017 12:40 PM, Lawrence Stewart wrote: > On 13/03/2017 04:30, 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. > I don't think so. I manually ran the compile step in > /usr/src/lib/libpam/modules/pam_exec replacing -o with -E per: > > cc -DOPENPAM_STATIC_MODULES -O2 -pipe -I/usr/src/contrib/openpam/include > -I/usr/src/lib/libpam -DOPENPAM_DEBUG -MD -MF.depend.pam_exec.o > -MTpam_exec.o -std=gnu99 -fstack-protector-strong -Wsystem-headers > -Werror -Wno-pointer-sign -Wno-empty-body -Wno-string-plus-int > -Wno-unused-const-variable -Wno-tautological-compare -Wno-unused-value > -Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion > -Wno-unused-local-typedef -Wno-address-of-packed-member -Wno-switch > -Wno-switch-enum -Wno-knr-promoted-parameter -Wno-parentheses > -Qunused-arguments -c pam_exec.c -E | vim - > > and the preprocessed code in question looks sane (included a few lines > of context either side): > > envlist = pam_getenvlist(pamh); > for (envlen = 0; envlist[envlen] != ((void *)0); ++envlen) > ; > nitems = sizeof(env_items) / sizeof(*env_items); > > nitems_rv = options->return_prog_exit_status ? 24 : 0; > tmp = reallocarray(envlist, envlen + nitems + 1 + nitems_rv + 1, > sizeof(*envlist)); > if (tmp == ((void *)0)) { > openpam_free_envlist(envlist); > return (PAM_BUF_ERR); > } OK, the nitems issue is cosmetical at this time. Are you getting PAM_BUF_ERR, in other words, is tmp NULL? We may be hitting some strict limit in reallocarray(). Pedro.