Date: Fri, 23 Apr 2004 08:00:38 -0700 (PDT) From: Bruce Evans <bde@zeta.org.au> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/18874: 32bit NFS servers export wrong negative values to 64bit clients Message-ID: <200404231500.i3NF0c9N085602@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/18874; it has been noted by GNATS. From: Bruce Evans <bde@zeta.org.au> To: "Tim J. Robbins" <tjr@freebsd.org> Cc: alex@big.endian.de, freebsd-gnats-submit@freebsd.org Subject: Re: kern/18874: 32bit NFS servers export wrong negative values to 64bit clients Date: Sat, 24 Apr 2004 00:58:48 +1000 (EST) On Fri, 23 Apr 2004, Tim J. Robbins wrote: > Synopsis: 32bit NFS servers export wrong negative values to 64bit clients > > State-Changed-From-To: suspended->patched > State-Changed-By: tjr > State-Changed-When: Fri Apr 23 06:15:28 PDT 2004 > State-Changed-Why: > Fixed in -current by mux@. The fix is wrong, since it breaks sending negative block counts to clients which understand them. BSD clients have supported this since at least 4.4BSD, but the code is broken for the nfsv3 case. This is fixed for small negative values in NetBSD by casting to a signed type as discussed in the followup to this PR. Sign extension bugs are closely related to overflow bugs here and elsewhere. Overflow occurs for block counts that represent byte counts >= 1TB (PR 56606). FreeBSD has wrong fixes for this. See the PR followup for more details. The bugs have now mostly moved to cvtstatfs(). They now also affect callers of the old statfs() family. Overflow problems in nfs_statfs() should have gone away, since 64-bit byte counts easily fit in 64-bit block counters after dividing by the 512. However, nfs still thinks that block counters are longs and it makes messes trying to get the 64-bit counts to fit in possibly-32-bit longs. See the followup to PR 56606 for a hopefully working version. This code needs to move to RELENG_4's nfs_statfs() and -current's cvtstatfs(). cvstatfs() is simple and broken. It attempts to truncate large values but has some sign bugs: % static void % cvtstatfs(td, nsp, osp) % struct thread *td; % struct statfs *nsp; % struct ostatfs *osp; % { % % bzero(osp, sizeof(*osp)); % osp->f_bsize = MIN(nsp->f_bsize, LONG_MAX); % osp->f_iosize = MIN(nsp->f_iosize, LONG_MAX); % osp->f_blocks = MIN(nsp->f_blocks, LONG_MAX); % osp->f_bfree = MIN(nsp->f_bfree, LONG_MAX); % osp->f_bavail = MIN(nsp->f_bavail, LONG_MAX); % osp->f_files = MIN(nsp->f_files, LONG_MAX); % osp->f_ffree = MIN(nsp->f_ffree, LONG_MAX); Mount of the fields in the new statfs struct are (bogusly) unsigned where they used to be signed. The clamps work for these. However, f_bavail and f_free are signed, so the above assignments to their "old" values overflow if their "new" values are < LONG_MIN. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200404231500.i3NF0c9N085602>