linux-ext4 - Re: [PATCH] ext4: Automatic setting of {INODE,BLOCK}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 8 Nov 2012 12:48:59 -0500
From:	Theodore Ts'o <tytso@....edu>
To:	Tomas Racek <tracek@...hat.com>
Cc:	linux-ext4@...r.kernel.org, lczerner@...hat.com
Subject: Re: [PATCH] ext4: Automatic setting of {INODE,BLOCK}_UNINIT flags

On Thu, Oct 25, 2012 at 09:25:43AM +0200, Tomas Racek wrote:
> When last inode from bg is freed, set the INODE_UNINIT flag, similarly
> when last block is freed, set BLOCK_UNINIT flag. This can speed up
> subsequent fsck run.
> 
> Signed-off-by: Tomas Racek <tracek@...hat.com>

Could you make the following enhancements to your patch?

1)  Only do this if ext4_has_group_desc_csum(sb) is true

2)  Check to make sure the inode bitmap has only one bit set (the inode
    to be freed).  Basically, I don't want to set the BLOCK/INODE_UNINIT
    based just on the block group descriptor count, in case it got corrupted
     --- what, me paranoid?

    If there is other inodes set, then we need to call ext4_error()
    since we know the file system has gotten corrupted.

3)  If we can set BLOCK/INODE_UNINIT, we can skip modifying the bitmap
    block (since we won't consult the bitmap block if uninit is set).
    So that means we can skip calling get_write_access(), modifying
    the bitmap, updating the checksum, and then calling
    handle_dirty_metadata.  In fact, we can call ext4_forget(), so we
    can drop it from the buffer cache, and if it had been modified during
    the current transaction --- as part of an rm -rf, for example ---
    we don't need to include the bitmap block in the transaction.

(2) makes this patch much safer, and (3) should more than make up for
the overhead of scanning the the bitmap.

Thanks!!!

						- Ted

P.S.  Although the block bitmap is metadata, you can pass 0 for
is_metadata, since we don't need to revoke the block --- that's only
needed for extent tree blocks, indirect blocks, or directory blocks,
which could potentially get reused as a data block.  This isn't an
issue for the bitmap blocks, so we don't need to include a revoke
record in the journal.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html