ext2fs: Validate file size when lengthening

[Steven McDonald]

Hi there,

ext2fs currently has #if 0 around the size check for lengthening a
file. This allows ftruncate(2) calls for very large files to succeed,
but result in a smaller file than was requested. I noticed the problem
while trying to create vmd(8) images on an ext2fs filesystem.

Below is a patch that fixes the problem, followed by a program that
demonstrates the problem.

I am not certain this patch is correct. A bit further down in
ext2fs_inode.c, we find:

        error = ext2fs_buf_alloc(oip, lbn, offset + 1, cred, &bp,
            aflags);
        if (error)
                return (error);
        (void)ext2fs_setsize(oip, length);

ext2fs_setsize will already return EFBIG if the requested size is too
large, and we then throw this information away with a void cast.
However, I am not sure how to properly clean up the effects of
ext2fs_buf_alloc in case of failure this late; it may be simpler to
perform the check earlier (as in the patch below).

I am happy to work on a better diff if this failure should be handled
differently.


Index: ext2fs_inode.c
===================================================================
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_inode.c,v
retrieving revision 1.58
diff -u -p -r1.58 ext2fs_inode.c
--- ext2fs_inode.c      19 Mar 2016 12:04:16 -0000      1.58
+++ ext2fs_inode.c      27 May 2017 19:11:48 -0000
@@ -250,10 +250,8 @@ ext2fs_truncate(struct inode *oip, off_t
         * value of osize is 0, length will be at least 1.
         */
        if (osize < length) {
-#if 0 /* XXX */
-               if (length > fs->fs_maxfilesize)
+               if (length > fs->e2fs_maxfilesize)
                        return (EFBIG);
-#endif
                offset = blkoff(fs, length - 1);
                lbn = lblkno(fs, length - 1);
                aflags = B_CLRBUF;


ftruncate.c
===================================================================
/*
 * This program demonstrates a bug in OpenBSD's ext2fs implementation.
 * The bug causes ftruncate(2) on ext2fs not to perform size checks
 * prior to lengthening a file, so that inconsistent behaviour results
 * if a file is ftruncate(2)'d beyond the maximum supported file size.
 *
 * The program is to be run with fd 3 directed to the file that should
 * be truncated.
 *
 * Expected behaviour with a maximum file size of 2 TiB minus one byte:
 *
 *   $ ./ftruncate 3>foo
 *   ftruncate: ftruncate 1T: success, file size is 1.0T
 *   ftruncate: ftruncate 2T: File too large
 *   ftruncate: ftruncate 4T: File too large
 *   ftruncate: ftruncate 9T: File too large
 *   ftruncate: ftruncate 10T: File too large
 *   ftruncate: ftruncate 100T: File too large
 *   ftruncate: ftruncate 900T: File too large
 *
 * Actual behaviour with the bug:
 *
 *   $ ./ftruncate 3>foo
 *   ftruncate: ftruncate 1T: success, file size is 1.0T
 *   ftruncate: ftruncate 2T: success, file size is 1.0T
 *   ftruncate: ftruncate 4T: success, file size is 1.0T
 *   ftruncate: ftruncate 9T: File too large
 *   ftruncate: ftruncate 10T: File too large
 *   ftruncate: ftruncate 100T: success, file size is 1.0T
 *   ftruncate: ftruncate 900T: success, file size is 1.0T
 */

#include <sys/stat.h>

#include <err.h>
#include <unistd.h>
#include <util.h>

#define FD 3

void do_ftruncate(char*);

int
main() {
        do_ftruncate("1T");
        do_ftruncate("2T");
        do_ftruncate("4T");
        do_ftruncate("9T");
        do_ftruncate("10T");
        do_ftruncate("100T");
        do_ftruncate("900T");
        return 0;
}

void
do_ftruncate(char* size) {
        long long r;
        struct stat sb;
        char rsize[FMT_SCALED_STRSIZE];

        if (scan_scaled(size, &r) != 0)
                err(1, "scan_scaled %s", size);

        if (ftruncate(FD, r) != 0) {
                warn("ftruncate %s", size);
                return;
        }

        if (fstat(3, &sb) != 0)
                err(1, "fstat");

        if (fmt_scaled(sb.st_size, rsize) != 0)
                err(1, "fmt_scaled %lld", sb.st_size);

        warnx("ftruncate %s: success, file size is %s", size, rsize);
}