Message ID | 20170508224757.94528-1-ebiggers3@gmail.com |
---|---|
State | Accepted, archived |
Headers | show |
On Mon, May 08, 2017 at 03:47:57PM -0700, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > When mk_hugefiles() was switched to use ext2fs_fallocate(), it was > accidentally changed to ignore the 'zero_hugefiles = false' setting, > which should cause hugefiles to be allocated without initializing their > contents. Fix this by only passing EXT2_FALLOCATE_ZERO_BLOCKS to > ext2fs_fallocate() when zero_hugefiles is true. Soooo... I don't know that I like the potential for stale data exposure, which predisposes me not to like this patch. On the other hand, mkfs isn't a frequently used program and it clearly supports this as a feature, so let's not have regressions. Acked-by: Darrick J. Wong <darrick.wong@oracle.com> --D > Fixes: 4f868703f6f2 ("libext2fs: use fallocate for creating journals and hugefiles") > Cc: Darrick J. Wong <darrick.wong@oracle.com> > Signed-off-by: Eric Biggers <ebiggers@google.com> > --- > misc/mk_hugefiles.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/misc/mk_hugefiles.c b/misc/mk_hugefiles.c > index 71a15c5d..049c6f41 100644 > --- a/misc/mk_hugefiles.c > +++ b/misc/mk_hugefiles.c > @@ -260,6 +260,7 @@ static errcode_t mk_hugefile(ext2_filsys fs, blk64_t num, > { > errcode_t retval; > struct ext2_inode inode; > + int falloc_flags; > > retval = ext2fs_new_inode(fs, 0, LINUX_S_IFREG, NULL, ino); > if (retval) > @@ -281,10 +282,11 @@ static errcode_t mk_hugefile(ext2_filsys fs, blk64_t num, > > if (ext2fs_has_feature_extents(fs->super)) > inode.i_flags |= EXT4_EXTENTS_FL; > - retval = ext2fs_fallocate(fs, > - EXT2_FALLOCATE_FORCE_INIT | > - EXT2_FALLOCATE_ZERO_BLOCKS, > - *ino, &inode, goal, 0, num); > + > + falloc_flags = EXT2_FALLOCATE_FORCE_INIT; > + if (zero_hugefile) > + falloc_flags |= EXT2_FALLOCATE_ZERO_BLOCKS; > + retval = ext2fs_fallocate(fs, falloc_flags, *ino, &inode, goal, 0, num); > if (retval) > return retval; > retval = ext2fs_inode_size_set(fs, &inode, num * fs->blocksize); > -- > 2.13.0.rc2.291.g57267f2277-goog >
On Mon, May 08, 2017 at 06:56:36PM -0700, Darrick J. Wong wrote: > > Soooo... I don't know that I like the potential for stale data exposure, > which predisposes me not to like this patch. We like causing mke2fs to take 24+ hours per high capacity HDD even less. :-) Especially when the only user of the huge file is a trusted storage server where all data is stored encrypted at rest with multiple layers of encryption[1], so forcing a zero write pass is really pointless. [1] https://cloud.google.com/security/encryption-at-rest/default-encryption/ Cheers, - Ted
diff --git a/misc/mk_hugefiles.c b/misc/mk_hugefiles.c index 71a15c5d..049c6f41 100644 --- a/misc/mk_hugefiles.c +++ b/misc/mk_hugefiles.c @@ -260,6 +260,7 @@ static errcode_t mk_hugefile(ext2_filsys fs, blk64_t num, { errcode_t retval; struct ext2_inode inode; + int falloc_flags; retval = ext2fs_new_inode(fs, 0, LINUX_S_IFREG, NULL, ino); if (retval) @@ -281,10 +282,11 @@ static errcode_t mk_hugefile(ext2_filsys fs, blk64_t num, if (ext2fs_has_feature_extents(fs->super)) inode.i_flags |= EXT4_EXTENTS_FL; - retval = ext2fs_fallocate(fs, - EXT2_FALLOCATE_FORCE_INIT | - EXT2_FALLOCATE_ZERO_BLOCKS, - *ino, &inode, goal, 0, num); + + falloc_flags = EXT2_FALLOCATE_FORCE_INIT; + if (zero_hugefile) + falloc_flags |= EXT2_FALLOCATE_ZERO_BLOCKS; + retval = ext2fs_fallocate(fs, falloc_flags, *ino, &inode, goal, 0, num); if (retval) return retval; retval = ext2fs_inode_size_set(fs, &inode, num * fs->blocksize);