bin/188715: int64 not handled right as arg on badsect(8), possible other issues lurking

[Bruce Evans]

The following reply was made to PR bin/188715; it has been noted by GNATS.

From: Bruce Evans <brde at optusnet.com.au>
To: Dirk-Willem van Gulik <dirkx at webweaving.org>
Cc: freebsd-gnats-submit at freebsd.org, freebsd-bugs at freebsd.org
Subject: Re: bin/188715: int64 not handled right as arg on badsect(8), possible
 other issues lurking
Date: Thu, 17 Apr 2014 23:43:03 +1000 (EST)

   This message is in MIME format.  The first part should be readable text,
   while the remaining parts are likely unreadable without MIME-aware tools.
 
 --0-1210473500-1397742183=:1829
 Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed
 Content-Transfer-Encoding: QUOTED-PRINTABLE
 
 On Thu, 17 Apr 2014, Dirk-Willem van Gulik wrote:
 
 >> Description:
 > Was trying to map out some bad blocks prior to temporarily read/empty 4 T=
 byte volume using =82badsect(8)=92 - and returing it.
 >
 > Was expecting to be able to put the sector # into badsect (e.g. 343263142=
 4 from below FSCK output).
 >
 > This gave me a bit of an odd:
 >
 > =09badsect: 3432631424: Result too large
 
 badsect's ABI uses mknod() and dev_t so it can only work up to 32 bits.
    (Before 4.4BSD, dev_t was only 16 bits, so badsect only works up to 16
    bits.  That is a whole 65536 sectors, or 33MB with 512-blocks.  This was
    almost enough in 1980.  However, the limit is on fs-blocks (fragments),
    not on 512-blocks, so the limit can be expanded a bit.  With the current
    default ffs block size of 32K, the fragment size is 4K, so the limit
    in 1984 would have been 256MB.  However, the default fragment size was
    512 until about 1995.)
 
 It also has bugs in its error checking, so it only works up to 31 bits
 on arches with 32-bit longs.  The above is one of them.  3432631424 doesn't
 fit in 31 bits, but it does fit in 32 bits.
 
 The bugs in the error checking are more serious than I thought, since
 the limit is on 512-blocks.  So on arches with 32-bit longs, the limit
 on the file system size is 2**31 * 2**9 =3D 1TB.  With 4K-frags it should
 be 2**32 * 2**12 =3D 16TB, so badsect should actually work on your 4TB
 file system.  It shouldn't be expected to work.  4TB disks are supposed
 to be replaced if they have a single uncorrectable error.
 
 There are related limits on file system size from the file system itself.
 ffs1 uses 32-bit signed block (fragment) numbers internally, so it shouldn'=
 t
 be affected by the 332-bit badsect ABI limit.
 
 > As the daddr_t seems to be a 64bit unsigned; I assumed that the:
 
 daddr_t is 64 bits signed.
 
 >
 > =09=09=09number =3D strtol(*argv, NULL, 0);
 >
 > was some legacy culprint - and changed it to a strtoll as the daddr_t you=
  are entering is an int 64.
 
 daddr_t used to be 32-bit signed, and badsect hasn't been maintained since
 before daddr_t became 64 bits.  Even 1GB disks are supposed to be replaced
 if they have a single uncorrectable error.
 
 > =09=09=09number =3D strtoll(*argv, NULL, 0);
 
 Ugh.  Long long should never be used.  Use intmax_t.
 
 badsect's buggy error checking involves many type errors.  It uses the
 system type daddr_t for 'number'.  This is correct.  But it also uses
 long, and in the above, long long, and in the clean version, intmax_t,
 for initializing 'number'.  I neglected to fix this when I updated
 badsect from 16 bits to 32 bits in 1995.  The update was incomplete.
 The result of strtoimax() should be assigned to a variable of type
 intmax_t.  It shouldn't be assigned to 'number' before checking that
 it fits.
 
 The type errors continue with badsect abusing daddr_t for the disk block
 number.  That was as correct as possible in 1994, and also in my fixes,
 because ffs also used this wrong type internally.  ffs now uses ufs_daddr1_=
 t
 and ufs_daddr_t internally.  daddr_t would work in badsect since it is
 64 bits signed and only 32 bits unsigned is needed, but it is logically
 wrong.
 
 > That gets it past that point; only to segv out on:
 >
 >     cg =3D dtog(fs, fsbn);
 >
 > =09/usr/include/ufs/ffs/fs.h:#define=09dtog(fs, d)=09((d) / (fs)->fs_fpg)
 > =09/usr/include/ufs/ffs/fs.h:#define=09dtogd(fs, d)=09((d) % (fs)->fs_fpg=
 )
 >
 > a bit later.  While fs is valid - it seems  fs->fs_fpg returns as =820=92=
  =97 why is this ?  Is geom too new ? Or is badsect too old/retired ?
 
 I don't see why that doesn't work.  fsdb is the fs-block (frag) number,
 and the types are now large enough although logically wrong.  Some of
 the differences for ffs2 are in macros, but the above macros are too
 simple to depend on the ffs version.  Maybe libufs messes up the
 initialization of all of 'fs'.
 
 > aacd1: hard error cmd=3Dread 4246326690-4246326721
 > .
 >
 > fsck(8):...
 > THE FOLLOWING DISK SECTORS COULD NOT BE READ: 3432631424, 3432631425, 343=
 2631426, 3432631427, 3432631428, 3432631429, 3432631430, 3432631431, 343263=
 1432, 3432631433, 3432631434, 3432631435, 3432631436, 3432631437, 343263143=
 8, 3432631439, 3432631440, 3432631441, 3432631442, 3432631443, 3432631444, =
 3432631445, 3432631446, 3432631447, 3432631448, 3432631449, 3432631450, 343=
 2631451, 3432631452, 3432631453, 3432631454, 3432631455,
 
 Perhaps too many for badsect.
 
 I haven't used badsect recently, but spent a lot of time rearranging whole
 partitions on a laptop drive to avoid a 20GB area with a few bad sectors.
 The rest of the disk worked suprisingly well.
 
 Bruce
 --0-1210473500-1397742183=:1829--