From owner-svn-src-all@freebsd.org Sat Jan 27 16:43:24 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5ED03ECAC1D for ; Sat, 27 Jan 2018 16:43:24 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from sonic302-35.consmr.mail.gq1.yahoo.com (sonic302-35.consmr.mail.gq1.yahoo.com [98.137.68.161]) (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 DCF446FA04 for ; Sat, 27 Jan 2018 16:43:23 +0000 (UTC) (envelope-from pfg@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1517071397; bh=mVkWvQ48KAXzKIiSk47WHQLKM52aigLEW0XCqiOX8uU=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From:Subject; b=XcnXrNS1G5Q04QRq898ZjgCYhewQR1q2MZHcFv3j0IBiDvgsT0hs15tO8eX/ZwmWtUrobehXFnVk4BpyfZain8jLexYYmDAKML5oSwXKKhwGWmQwQb5ttn3KdAIWbFSGH7hfkWMFqPfIHP1cG0NKHDBnMJfkwguTnsyQFF4F1NymqGtv8jQE+k+7fXzLtpiekmmhPohSJo9IxI9Tuflrki2i2A/WqJ2ulHTzptAXCHrF0PgCacRvM0BDcZTquL/+TtZMRvH0Kx8ND/UkHokaOAxSYaum2o+Z5vfOFvILpDGGgfQJMJVpoOYW70lv5SwNKXweMHxe7dnlhZAlsDLzlA== X-YMail-OSG: sLogx1EVM1ln1SLXmsLwP9ITW_35W7OcNFxGPORuWXzsNGVsYsflJonE1AUtzLU mSPvBmDnJDNvgu9Fe1Hm6wygnIx8IyBrvXbHiYG9rOSns6UjR2fGhs05gboFOD9hxbASUO8NeB97 ufIjrjI._Jy3m9LLLbvuFuao_BC7wzC3fTjLNGzefVHvy_Dd1h00g9e98XpFPzZh_OvcimhxG6GR 4LmLpe1S0.XmJCujJJr9TSrTl35UKoHTcFoig_FvAafhfr.i2IDesV.M.5nrqm0n17kJ5JQPW7gR BXge..CYwNJjlW5XWz1SClrqnXPHeHtI.XmDnAcQP3G5xcAR8PKQUJ_R9yaH3oWNlUq6va4EgUep ohyr5ouhTOMlOcY44QkyL6lVgsiRFopB1x1q.Z4.13z_Yhx4Hced4fXVBNkliF9l7AsLB6ym0TsD O_D0uaTS9BJhLj2Zv5V4cI4531uiYayN0neaS4oK9FFg5RFTtVq89g75xuzN31lYOqerhROnWqpo LhZw_izJztg-- Received: from sonic.gate.mail.ne1.yahoo.com by sonic302.consmr.mail.gq1.yahoo.com with HTTP; Sat, 27 Jan 2018 16:43:17 +0000 Received: from smtp103.rhel.mail.gq1.yahoo.com (EHLO [192.168.0.8]) ([68.180.227.11]) by smtp402.mail.gq1.yahoo.com (JAMES SMTP Server ) with ESMTPA ID bc32d8913b75c14c896760960170103e; Sat, 27 Jan 2018 16:43:14 +0000 (UTC) Subject: Re: svn commit: r328479 - in head/sys: fs/ext2fs ufs/ufs To: Konstantin Belousov Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201801271533.w0RFXq0K057921@repo.freebsd.org> <20180127160340.GB55707@kib.kiev.ua> From: Pedro Giffuni Message-ID: <5335b45b-f527-c388-91fc-243d67511534@FreeBSD.org> Date: Sat, 27 Jan 2018 11:43:13 -0500 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180127160340.GB55707@kib.kiev.ua> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 27 Jan 2018 16:43:24 -0000 On 01/27/18 11:03, Konstantin Belousov wrote: > On Sat, Jan 27, 2018 at 03:33:52PM +0000, Pedro F. Giffuni wrote: >> Author: pfg >> Date: Sat Jan 27 15:33:52 2018 >> New Revision: 328479 >> URL: https://svnweb.freebsd.org/changeset/base/328479 >> >> Log: >> {ext2|ufs}_readdir: Set limit on valid ncookies values. >> >> Sanitize the values that will be assigned to ncookies so that we ensure >> they are sane and we can handle them. >> >> Let ncookies signed as it was before r328346. The valid range is such >> that unsigned values are not required and we are not able to avoid at >> least one cast anyways. >> >> Hinted by: bde >> >> Modified: >> head/sys/fs/ext2fs/ext2_lookup.c >> head/sys/ufs/ufs/ufs_vnops.c >> >> Modified: head/sys/fs/ext2fs/ext2_lookup.c >> ============================================================================== >> --- head/sys/fs/ext2fs/ext2_lookup.c Sat Jan 27 13:46:55 2018 (r328478) >> +++ head/sys/fs/ext2fs/ext2_lookup.c Sat Jan 27 15:33:52 2018 (r328479) >> @@ -145,14 +145,18 @@ ext2_readdir(struct vop_readdir_args *ap) >> off_t offset, startoffset; >> size_t readcnt, skipcnt; >> ssize_t startresid; >> - u_int ncookies; >> + int ncookies; >> int DIRBLKSIZ = VTOI(ap->a_vp)->i_e2fs->e2fs_bsize; >> int error; >> >> if (uio->uio_offset < 0) >> return (EINVAL); >> ip = VTOI(vp); >> + if (uio->uio_resid < 0) >> + uio->uio_resid = 0; >> if (ap->a_ncookies != NULL) { >> + if (uio->uio_resid > MAXPHYS) >> + uio->uio_resid = MAXPHYS; >> ncookies = uio->uio_resid; >> if (uio->uio_offset >= ip->i_size) >> ncookies = 0; >> >> Modified: head/sys/ufs/ufs/ufs_vnops.c >> ============================================================================== >> --- head/sys/ufs/ufs/ufs_vnops.c Sat Jan 27 13:46:55 2018 (r328478) >> +++ head/sys/ufs/ufs/ufs_vnops.c Sat Jan 27 15:33:52 2018 (r328479) >> @@ -2170,7 +2170,7 @@ ufs_readdir(ap) >> off_t offset, startoffset; >> size_t readcnt, skipcnt; >> ssize_t startresid; >> - u_int ncookies; >> + int ncookies; >> int error; >> >> if (uio->uio_offset < 0) >> @@ -2178,7 +2178,11 @@ ufs_readdir(ap) >> ip = VTOI(vp); >> if (ip->i_effnlink == 0) >> return (0); >> + if (uio->uio_resid < 0) >> + uio->uio_resid = 0; >> if (ap->a_ncookies != NULL) { >> + if (uio->uio_resid > MAXPHYS) >> + uio->uio_resid = MAXPHYS; > You just break nfs server. > > Look at nfsrvd_readdir() call to VOP_READDIR. Almost all code which calls > VOP_READDIR() memoize the value of uio_resid before and after the call and > interpret the difference as the amount of data returned. > > I said above that only nfs server is broken, because only the server uses > cookies, otherwise your patch would break everything. Ugh, yes .. I completely missed the fact that uio is a pointer to the real thing, not a local copy. This still should never go off limits but it is certainly wrong. I reverted it sorry. Pedro.