linux-ext4 - Re: Trouble mounting metadata_csum ext4 filesystems with v4.7.x after c9274d891869880648c4ee9365df3ecc7ba2e285: not enough inode bytes checksummed?

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>] [day] [month] [year] [list]

Message-id: <1153903080.1652319.1474378558082.JavaMail.weblogic@ep1ml103c>
Date:   Tue, 20 Sep 2016 13:35:58 +0000
From:   정대호 <daeho.jeong@...sung.com>
To:     Darrick J. Wong <darrick.wong@...cle.com>,
        Nix <nix@...eri.org.uk>,
        Theodore Ts'o <tytso@....edu>
Cc:     Linux FS Maling List 
        <linux-fsdevel@...r.kernel.org>,
        정대호 <daeho.jeong@...sung.com>,
        linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: Trouble mounting metadata_csum ext4 filesystems with v4.7.x after c9274d891869880648c4ee9365df3ecc7ba2e285: not enough inode bytes checksummed?

Hi, Sorry to bother you.

> Basically, a 128-byte inode inside a filesystem that allocated 256 bytes
> for each inode.  As you point out, the old code would checksum the entire
> allocated space, whether or not the inode core used it.  Obviously, you
> want this since inline extended attributes live in that space:
 
> csum = ext4_chksum(sbi, ei->i_csum_seed, (__u8 *)raw,
>                   EXT4_INODE_SIZE(inode->i_sb));
 
> The new code, on the other hand, carefully checksums around the
> i_checksum fields and only bothers to checksum the space between the end
> of i_checksum_hi and the end of the allocated space if the inode core is
> big enough to store i_checksum_hi.  Since we allocated 256 bytes for
> each inode, we checksum the first two bytes after byte 128
> (EXT4_GOOD_OLD_INODE_SIZE), but then we see that i_extra_size == 0 so we
> never bother to checksum anything after that.  This is of course wrong
> since we no longer checksum the xattr space and we've deviated from the
> pre-4.7.4 (documented) on-disk format.

Oops. I had overlooked the case of that i_extra_size is less than 4.
I misunderstood the previous Darrick's codes calculating inode checksum value.
Sorry about that. :-(
 
> if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) {
>        offset = offsetof(struct ext4_inode, i_checksum_hi);
>        csum = ext4_chksum(sbi, csum, (__u8 *)raw +
>                           EXT4_GOOD_OLD_INODE_SIZE,
>                           offset - EXT4_GOOD_OLD_INODE_SIZE);
>        if (EXT4_FITS_IN_INODE(raw, ei, i_checksum_hi)) {
>                csum = ext4_chksum(sbi, csum, (__u8 *)&dummy_csum,
>                                   csum_size);
>                offset += csum_size;
>        }
>        csum = ext4_chksum(sbi, csum, (__u8 *)raw + offset,
>                           EXT4_INODE_SIZE(inode->i_sb) - offset);
> }
> 
> Can you give that a try?

Darrick, your modification looks good enough to me.

Thank you, guys.