On Tue, Mar 27, 2018 at 12:46:03PM +0200, Tobias Stoeckmann wrote:
> Resending this old diff. Adjusted to apply with -current source:
>
> Illegal fragmentation block sizes can trigger division by zero in the
> disklabel and fsck_ffs tools.
>
> See this sequence of commands to reproduce:
>
> # dd if=/dev/zero of=nofrag.iso bs=1M count=1
> # vnconfig vnd0 nofrag.iso
> # disklabel -e vnd0
>
> Create 'a' and set fsize = bsize = 1. A line like this one will do
> a: 2048 0 4.2BSD 1 1 1
>
> # fsck_ffs vnd0a
> ** /dev/vnd0a (vnd0a)
> BAD SUPER BLOCK: MAGIC NUMBER WRONG
> Floating point exception (core dumped)
> # disklabel -E vnd0
> Label editor (enter '?' for help at any prompt)
> > m a
> offset: [0]
> size: [2048]
> FS type: [4.2BSD]
> Floating point exception (core dumped)
> # vnconfig -u vnd0
>
> A fragmentation (block) size smaller than a sector size is not valid
> while using "disklabel -E", and really doesn't make sense. While
> using "disklabel -e", not all validation checks are performed, which
> makes it possible to enter invalid values.
>
> If "disklabel -E" is used without the expert mode, fragmentation sizes
> cannot be changed and will be just accepted from the parsed disklabel,
> resulting in a division by zero if they are too small.
>
> And the same happens in fsck_ffs. Instead of coming up with a guessed
> value in fsck_ffs, I think it's better to simply fail and let the user
> fix the disklabel. After all, it shouldn't be fsck_ffs's duty to fix
> faulty values outside the filesystem.
This looks from reading the diff, but "fragmentation size" is not the
correct term, see inline.
-Otto
>
> Index: sbin/disklabel/disklabel.c
> ===================================================================
> RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
> retrieving revision 1.227
> diff -u -p -u -p -r1.227 disklabel.c
> --- sbin/disklabel/disklabel.c 25 Feb 2018 17:24:44 -0000 1.227
> +++ sbin/disklabel/disklabel.c 27 Mar 2018 10:42:59 -0000
> @@ -1100,9 +1100,24 @@ gottype:
>
> case FS_BSDFFS:
> NXTNUM(fsize, fsize, &errstr);
> - if (fsize == 0)
> + if (fsize < lp->d_secsize ||
> + (fsize % lp->d_secsize) != 0) {
> + warnx("line %d: "
> + "bad fragmentation size: %s",
Should be "fragment size"
> + lineno, cp);
> + errors++;
> break;
> + }
> NXTNUM(v, v, &errstr);
> + if (v < fsize || (fsize != v &&
> + fsize * 2 != v && fsize * 4 != v &&
> + fsize * 8 != v)) {
> + warnx("line %d: "
> + "bad block size: %s",
> + lineno, cp);
> + errors++;
> + break;
> + }
> pp->p_fragblock =
> DISKLABELV1_FFS_FRAGBLOCK(fsize, v / fsize);
> NXTNUM(pp->p_cpg, pp->p_cpg, &errstr);
> Index: sbin/disklabel/editor.c
> ===================================================================
> RCS file: /cvs/src/sbin/disklabel/editor.c,v
> retrieving revision 1.327
> diff -u -p -u -p -r1.327 editor.c
> --- sbin/disklabel/editor.c 8 Mar 2018 22:05:17 -0000 1.327
> +++ sbin/disklabel/editor.c 27 Mar 2018 10:42:59 -0000
> @@ -2069,16 +2069,16 @@ get_bsize(struct disklabel *lp, int part
> if (pp->p_fstype != FS_BSDFFS)
> return (0);
>
> + frag = DISKLABELV1_FFS_FRAG(pp->p_fragblock);
> + fsize = DISKLABELV1_FFS_FSIZE(pp->p_fragblock);
> +
> /* Avoid dividing by zero... */
> - if (pp->p_fragblock == 0)
> - return(1);
> + if (frag * fsize < lp->d_secsize)
> + return (1);
>
> if (!expert)
> goto align;
>
> - fsize = DISKLABELV1_FFS_FSIZE(pp->p_fragblock);
> - frag = DISKLABELV1_FFS_FRAG(pp->p_fragblock);
> -
> for (;;) {
> ui = getuint64(lp, "block size",
> "Size of ffs blocks. 1, 2, 4 or 8 times ffs fragment size.",
> @@ -2119,8 +2119,7 @@ align:
> orig_size = DL_GETPSIZE(pp);
> orig_offset = DL_GETPOFFSET(pp);
>
> - bsize = (DISKLABELV1_FFS_FRAG(pp->p_fragblock) *
> - DISKLABELV1_FFS_FSIZE(pp->p_fragblock)) / lp->d_secsize;
> + bsize = (frag * fsize) / lp->d_secsize;
> if (DL_GETPOFFSET(pp) != starting_sector) {
> /* Can't change offset of first partition. */
> adj = bsize - (DL_GETPOFFSET(pp) % bsize);
> Index: sbin/fsck_ffs/setup.c
> ===================================================================
> RCS file: /cvs/src/sbin/fsck_ffs/setup.c,v
> retrieving revision 1.64
> diff -u -p -u -p -r1.64 setup.c
> --- sbin/fsck_ffs/setup.c 5 Jan 2018 09:33:47 -0000 1.64
> +++ sbin/fsck_ffs/setup.c 27 Mar 2018 10:42:59 -0000
> @@ -636,6 +636,10 @@ calcsb(char *dev, int devfd, struct fs *
> fs->fs_frag = DISKLABELV1_FFS_FRAG(pp->p_fragblock);
> fs->fs_fpg = pp->p_cpg * fs->fs_frag;
> fs->fs_bsize = fs->fs_fsize * fs->fs_frag;
> + if (fs->fs_bsize < lp->d_secsize) {
> + pfatal("%s: INVALID BLOCK FRAGMENTATION SIZE\n", dev);
This should be e.g. "INVALID BLOCK OR FRAGMENT SIZE IN DISKLABEL"
> + return (0);
> + }
> fs->fs_nspf = DL_SECTOBLK(lp, fs->fs_fsize / lp->d_secsize);
> for (fs->fs_fsbtodb = 0, i = NSPF(fs); i > 1; i >>= 1)
> fs->fs_fsbtodb++;
