From owner-freebsd-fs@FreeBSD.ORG Mon Feb 4 02:35:11 2013 Return-Path: Delivered-To: freebsd-fs@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 A5884C49 for ; Mon, 4 Feb 2013 02:35:11 +0000 (UTC) (envelope-from giffunip@yahoo.com) Received: from nm20.bullet.mail.bf1.yahoo.com (nm20.bullet.mail.bf1.yahoo.com [98.139.212.179]) by mx1.freebsd.org (Postfix) with SMTP id 23E84833 for ; Mon, 4 Feb 2013 02:35:10 +0000 (UTC) Received: from [98.139.212.147] by nm20.bullet.mail.bf1.yahoo.com with NNFMP; 04 Feb 2013 02:32:34 -0000 Received: from [98.139.215.254] by tm4.bullet.mail.bf1.yahoo.com with NNFMP; 04 Feb 2013 02:32:34 -0000 Received: from [127.0.0.1] by omp1067.mail.bf1.yahoo.com with NNFMP; 04 Feb 2013 02:32:34 -0000 X-Yahoo-Newman-Property: ymail-3 X-Yahoo-Newman-Id: 436143.10915.bm@omp1067.mail.bf1.yahoo.com Received: (qmail 68588 invoked by uid 60001); 4 Feb 2013 02:32:34 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s1024; t=1359945154; bh=14RC7nOYdMul2SlNDTso11jKS8EjxVfiFk8xEGdHNOs=; h=X-YMail-OSG:Received:X-Rocket-MIMEInfo:X-RocketYMMF:X-Mailer:References:Message-ID:Date:From:Reply-To:Subject:To:Cc:In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding; b=qw6rU9L/T0AEhW9hluwDhmwynqMxzPq+3xvOAcjyO4DrKsb63FIDCNYU1cMq+WHC9C3ZuMtdlZ/WFIL8Dj+pkFMWDoshj/3I5AlcIpyK0kQZyWFjxixB1XtJshzoDS5q8Sa5L/Jp2V2hnQkTQIyXxng5vTLurJuaNQ5jlinD2IE= DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=s1024; d=yahoo.com; h=X-YMail-OSG:Received:X-Rocket-MIMEInfo:X-RocketYMMF:X-Mailer:References:Message-ID:Date:From:Reply-To:Subject:To:Cc:In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding; b=O8Asd/Iw+ckmg0yBo7pL0LMHoSfh/90o3d0d1gKBt1ZFC3k2/o08L64pLt/5bbf/t4ixs6dfHuO5wxRHBdTsWDdnMYaXTGRi87oLBQxlm56pOhQXu+UBNYod8dJoBWJi9hfF5sMoppCss7Pi5hIiB5ihdoyIfydN4C1lnsCmfQ4=; X-YMail-OSG: vMbNGr0VM1kLe.G5cXdqJ.4W9J8HNAZmyPII2nZvHbW_0Fz pH2CWz5M8XVxQwN4CeQktL_TZzQYCDqnWiI1eClElMfVC6Wi7Whn1JQ5gj.h 12.9MuXrLuMoJFChk.6y1Da7cbr6ZzEiOCIe6G9G0p2TRA_QqUcVlYjxRbqH pibz1cTx92fZM37.TpkG_dzNagvLxQtiVi3AmaVF9_j2VZlEWZ0kqy2z9w1j GhqvOxZH.KLWBVhsqJnJ84tj8jn8MxPSlY06dZs8oIuPJF0lG5h5DGmYWIa3 Y3FJcy4Nbd3UI6Kp3pYV7xDhQSGbCSwSrUi7BTQ7867.QwTPf69YTE9q_S7E RBYbxS8uJdCy5UjWy0LqQbWSJjEbGjlY4IRySnYGuu5xi_fNFzaqS_KCdgdu oq9gTUKLcvqUfqe01qWYRxmqkJGlUZCJ4to285tWdqwRjZ_BMhxobNEjINkH kiO_daqgVAXqQRiQFHNl4z9OhTwyn_nDGRvJXWvSwcOzmEnUG53NoOan0DLT VjAQu_RAyuz7lM6fYLA-- Received: from [200.118.157.7] by web162105.mail.bf1.yahoo.com via HTTP; Sun, 03 Feb 2013 18:32:34 PST X-Rocket-MIMEInfo: 001.001, KE1vdmluZyB0aGUgZGlzY3Vzc2lvbiB0byBmcmVlYnNkLWZzKQoKSGVsbG87CgoKLS0tLS0gTWVzc2FnZ2lvIG9yaWdpbmFsZSAtLS0tLQo.IERhOiBCcnVjZSBFdmFuc8KgCj4gCi4uLgo.PiAgSnVzdCBhIG5vdGUgdGhhdCBjbGFuZyBhY3R1YWxseSB3YXJuZWQgYWJvdXQgdGhpcyBvbmUuCj4.ICBJdCBoYXMgYSBmZXcgbW9yZSBzaW1pbGFyIHdhcm5pbmdzIGZvciB1ZnMvZmZzIGNvZGUuCj4gCj4gSSB3b25kZXJlZCBob3cgdGhlIERJUCBtYWNybyBoaWQgdGhlIHdhcm5pbmcuCj7CoAoKVGhlIGNvbXBhcmkBMAEBAQE- X-RocketYMMF: giffunip X-Mailer: YahooMailWebService/0.8.131.499 References: <201302031716.r13HGXNP060303@svn.freebsd.org> <510E9D47.2030403@FreeBSD.org> <20130204062149.U2673@besplex.bde.org> Message-ID: <1359945154.62069.YahooMailNeo@web162105.mail.bf1.yahoo.com> Date: Sun, 3 Feb 2013 18:32:34 -0800 (PST) From: Pedro Giffuni Subject: Re: svn commit: r246289 - head/sys/ufs/ffs To: Bruce Evans , Andriy Gapon In-Reply-To: <20130204062149.U2673@besplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Cc: "freebsd-fs@freebsd.org" , Kirk McKusick X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: Pedro Giffuni List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Feb 2013 02:35:11 -0000 (Moving the discussion to freebsd-fs)=0A=0AHello;=0A=0A=0A----- Messaggio o= riginale -----=0A> Da: Bruce Evans=A0=0A> =0A...=0A>> Just a note that cla= ng actually warned about this one.=0A>> It has a few more similar warnings= for ufs/ffs code.=0A> =0A> I wondered how the DIP macro hid the warning.= =0A>=A0=0A=0AThe comparison is perfectly legal for UFS1 so perhaps=0Agcc gi= ves the "benefit of the doubt" to avoid false positives.=0A=0A>>> =A0 bloc= ks being freed against the value i_blocks. If the number of=0A>>> =A0 bloc= ks being freed exceeds i_blocks, just set i_blocks to zero.=0A>>> =0A>>> = =A0 Reported by: Pedro Giffuni (pfg@)=0A>>> =A0 MFC after:=A0 2 weeks=0A= > =0A> Perhaps the larger bugs pointed to this warning were lost in transla= tion:=0A> - di_blocks overflows for ffs1.=A0 This is now physically possibl= e.=0A> =A0 di_blocks has type int32_t, as required to match st_blocks in th= e old=0A> =A0 struct stat (both will overflow at the same point).=A0 Since = di_blocks=0A> =A0 counts DEV_BSIZE-blocks, overflow occurs at file size abo= ut 1TB for=0A> =A0 files without many holes.=0A> - st_blocks overflows for = the old stat() syscall.=A0 For the new stat()=0A> =A0 syscall, st_blocks ha= s type blkcnt_t =3D int64_t, so overflow is not=0A> =A0 physically possible= .=A0 But cvtstat() silently overflows when the=0A> =A0 64-bit st_blocks doe= sn't fit in the 32-bit one.=0A> - di_blocks for ffs2 has type uint64_t.=A0 = This has a sign mismatch with=0A> =A0 both the ffs1 di_blocks and blkcnt_t.= =A0 blkcnt_t is specified to be=0A> =A0 signed.=A0 i_blocks for ffs has the= same type as di_blocks for ffs2=0A> =A0 (unsigned ...), so it is mismatche= d too.=A0 The sign mismatches should=0A> =A0 cause more compiler warnings.= =A0 These would point to further overflow=0A> =A0 possibilities.=A0 However= , overflow is physically impossible even for=0A> =A0 va_bytes =3D i_blocks = * DEV_BSIZE.=A0 So there are no extra overflows=0A> =A0 from the sign misma= tches.=A0 Using the unsigned types just asks for=0A> =A0 more sign extensio= n bugs like the one fixed here.=0A> =0A> ext2fs has the same bug.=A0 It is = unnecessary in ext2fs, since there are=0A> no fragments so i_blocks can jus= t count in units of fs blocks and these=0A> are naturally limited by the fs= block type.=A0 The fs block type is=0A> uint32_t in ext2fs and int32_t in = ffs1.=A0 So ext2fs really needs an=0A> unsigned type for i_blocks to get an= extra bit without going to 64=0A> bits.=A0 shell-init: could not get curre= nt directory: No such file or=0A> directory.=A0 Avoiding overflow depends o= n st_blocks having more than 32+9=0A> value bits, so 32-bit stat() could st= ill overflow and needs a clamp.=0A> However, the disk format is to store i_= count in DEV_BSIZE units, so=0A> this doesn't work -- it has the same overf= low problem as cvtstat(),=0A> at 32 bits instead of 31.=0A> =0A> Limiting t= he file size to ~1TB is not a good fix for this, since 1TB=0A> non-sparse f= iles are not very common or useful, and the limit would=0A> also apply to s= parse files.=A0 So block allocators should check that=0A> i_blocks won't ov= erflow before increasing it.=0A>=A0=0A=0ASurely not anywhere near a complet= e solution but perhaps it wouldn't=0Abe incompatible=A0to change i_blocks a= nd friends to be unsigned in UFS1.=0AThat is=A0something that remains to be= completed in ext2fs, but according=0Ato fsx there=A0are bigger=A0problems = there at this time.=0A=0APedro.