e2fsck: always probe filesystem blocksize with simple io_manager

Combining superblock (-b) with undo file (-z) fails iff the block size
is not specified (-B) and is different from the first blocksize probed
in try_open_fs (1k).  The reason is as follows:

try_open_fs() will probe different blocksizes if none is provided on
the command line. It is done by opening and closing the filesystem
until it finds a blocksize that makes sense. This is fine for all
io_managers, but undo_io creates the undo file with that blocksize
during ext2fs_open.  Once try_open_fs realizes it had the wrong
blocksize and retries with a different blocksize, undo_io will read
the previously created file and think it's corrupt for this
filesystem.

Ideally, undo_io would know this is a probe and would fix the undo file.
It is not simple, though, because it would require undo_io to know the
file was just created by the probe code, since an undo file survives
through different fsck sessions.  We'd have to pass this information
around somehow.  This seems like a complex change to solve a corner
case.

Instead, this patch changes the blocksize probe to always use the
unix_io_manager. This way, we safely probe for the blocksize without
side effects.  Once the blocksize is known, we can safely reopen the
filesystem under the proper io_manager.

An easily reproducer for this issue (from Ted, adapted by me) is:

 mke2fs -b 4k -q -t ext4 /tmp/foo.img 2G
 e2fsck -b 32768 -z /tmp/undo /tmp/foo.img

Reported-by: Peter Urbanec <linux-ext4.vger.kernel.org@urbanec.net>
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index af57e6a..3708a4d 100644 (file)
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -1171,25 +1171,32 @@ static errcode_t try_open_fs(e2fsck_t ctx, int flags, io_manager io_ptr,
        errcode_t retval;
 
        *ret_fs = NULL;
-       if (ctx->superblock && ctx->blocksize) {
-               retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options,
-                                     flags, ctx->superblock, ctx->blocksize,
-                                     io_ptr, ret_fs);
-       } else if (ctx->superblock) {
-               int blocksize;
-               for (blocksize = EXT2_MIN_BLOCK_SIZE;
-                    blocksize <= EXT2_MAX_BLOCK_SIZE; blocksize *= 2) {
-                       if (*ret_fs) {
-                               ext2fs_free(*ret_fs);
-                               *ret_fs = NULL;
+
+       if (ctx->superblock) {
+               unsigned long blocksize = ctx->blocksize;
+
+               if (!blocksize) {
+                       for (blocksize = EXT2_MIN_BLOCK_SIZE;
+                            blocksize <= EXT2_MAX_BLOCK_SIZE; blocksize *= 2) {
+
+                               retval = ext2fs_open2(ctx->filesystem_name,
+                                                     ctx->io_options, flags,
+                                                     ctx->superblock, blocksize,
+                                                     unix_io_manager, ret_fs);
+                               if (*ret_fs) {
+                                       ext2fs_free(*ret_fs);
+                                       *ret_fs = NULL;
+                               }
+                               if (!retval)
+                                       break;
                        }
-                       retval = ext2fs_open2(ctx->filesystem_name,
-                                             ctx->io_options, flags,
-                                             ctx->superblock, blocksize,
-                                             io_ptr, ret_fs);
-                       if (!retval)
-                               break;
+                       if (retval)
+                               return retval;
                }
+
+               retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options,
+                                     flags, ctx->superblock, blocksize,
+                                     io_ptr, ret_fs);
        } else
                retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options,
                                      flags, 0, 0, io_ptr, ret_fs);