All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Dilger <aedilger@gmail.com>
To: "Darrick J. Wong" <djwong@us.ibm.com>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>,
	Theodore Tso <tytso@mit.edu>,
	"Darrick J. Wong" <djwong@us.ibm.com>,
	Sunil Mushran <sunil.mushran@oracle.com>,
	Amir Goldstein <amir73il@gmail.com>,
	Andi Kleen <andi@firstfloor.org>, Mingming Cao <cmm@us.ibm.com>,
	Joel Becker <jlbec@evilplan.org>,
	"linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
	Coly Li <colyli@gmail.com>
Subject: Re: [PATCH 06/37] libext2fs: Add inode checksum support
Date: Sun, 4 Sep 2011 11:59:44 -0600	[thread overview]
Message-ID: <13B69EAD-7ABF-437C-95A4-3FA80ED98562@gmail.com> (raw)
In-Reply-To: <20110901003549.1176.81379.stgit@elm3c44.beaverton.ibm.com>

On 2011-08-31, at 6:35 PM, "Darrick J. Wong" <djwong@us.ibm.com> wrote:

> This patch adds the ability for the libext2fs functions to read and write the
> inode checksum.  It also fixes a few fields that were omitted from the byte
> swapping routines.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
> lib/ext2fs/csum.c         |   59 +++++++++++++++++++++++++++++++++++++++++++++
> lib/ext2fs/ext2_err.et.in |    9 +++++++
> lib/ext2fs/ext2_fs.h      |    4 ++-
> lib/ext2fs/ext2fs.h       |    6 +++++
> lib/ext2fs/inode.c        |   20 +++++++++++++++
> lib/ext2fs/swapfs.c       |   10 ++++++--
> 6 files changed, 104 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/lib/ext2fs/csum.c b/lib/ext2fs/csum.c
> index 2fece68..57adc4c 100644
> --- a/lib/ext2fs/csum.c
> +++ b/lib/ext2fs/csum.c
> @@ -29,6 +29,65 @@
> #define STATIC static
> #endif
> 
> +__u32 ext2fs_inode_csum(ext2_filsys fs, ext2_ino_t inum,
> +            struct ext2_inode_large *inode)
> +{
> +    struct ext2_inode_large *desc = inode;
> +    int offset = offsetof(struct ext2_inode_large, i_checksum);
> +    int extra_size = inode->i_extra_isize;
> +    size_t size = fs->super->s_inode_size;
> +    __u32 crc = 0;
> +
> +    if (size < EXT2_GOOD_OLD_INODE_SIZE + extra_size)
> +        printf("ERROR: inode %d size %d != extra_size %d!\n", inum,
> +               size, extra_size + EXT2_GOOD_OLD_INODE_SIZE);

This shouldn't have to be checked for every inode checksum. If the checksum is covering the whole inode size then this is a constant size, and e2fsck is already checking the value of i_extra_isize. 

> +    if (fs->super->s_creator_os != EXT2_OS_LINUX)
> +        return 0;
> +
> +    if (!EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
> +                    EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
> +
> +        return 0;
> +
> +#ifdef WORDS_BIGENDIAN
> +    char buf[EXT2_INODE_CORE_SIZE(fs->super)];
> +    struct ext2_inode_large *swabinode = (struct ext2_inode_large *)buf;
> +
> +    /* Have to swab back to little-endian to do the checksum */
> +    memcpy(swabinode, inode, size);
> +    ext2fs_swap_inode_full(fs, swabinode, swabinode, 1, size);
> +    desc = swabinode;
> +#endif
> +    inum = ext2fs_cpu_to_le32(inum);
> +    crc = crc32c_le(~0, fs->super->s_uuid, sizeof(fs->super->s_uuid));
> +    crc = crc32c_le(crc, (char *)&inum, sizeof(inum));
> +    crc = crc32c_le(crc, (char *)desc, offset);
> +    offset += sizeof(inode->i_checksum); /* skip checksum */
> +    crc = crc32c_le(crc, (char *)desc + offset,
> +               EXT2_GOOD_OLD_INODE_SIZE + extra_size - offset);
> +    return crc;
> +}
> +
> +int ext2fs_inode_csum_verify(ext2_filsys fs, ext2_ino_t inum,
> +                 struct ext2_inode_large *inode)
> +{
> +    if (fs->super->s_creator_os == EXT2_OS_LINUX &&
> +        EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
> +                       EXT4_FEATURE_RO_COMPAT_METADATA_CSUM) &&
> +        (inode->i_checksum != ext2fs_inode_csum(fs, inum, inode)))
> +        return 0;
> +    return 1;
> +}
> +
> +void ext2fs_inode_csum_set(ext2_filsys fs, ext2_ino_t inum,
> +               struct ext2_inode_large *inode)
> +{
> +    if (fs->super->s_creator_os != EXT2_OS_LINUX)
> +        return;
> +    inode->i_checksum = ext2fs_inode_csum(fs, inum, inode);
> +}
> +
> STATIC __u16 ext2fs_group_desc_csum(ext2_filsys fs, dgrp_t group)
> {
>    __u16 crc = 0;
> diff --git a/lib/ext2fs/ext2_err.et.in b/lib/ext2fs/ext2_err.et.in
> index 995ddc3..31c8fe1 100644
> --- a/lib/ext2fs/ext2_err.et.in
> +++ b/lib/ext2fs/ext2_err.et.in
> @@ -422,4 +422,13 @@ ec    EXT2_NO_MTAB_FILE,
> ec    EXT2_ET_CANT_USE_LEGACY_BITMAPS,
>    "Filesystem too large to use legacy bitmaps"
> 
> +ec    EXT2_ET_INODE_CSUM_INVALID,
> +    "Inode checksum is incorrect"
> +
> +ec    EXT2_ET_INODE_CORRUPT,
> +    "Inode checksum indicates corruption"
> +
> +ec    EXT2_ET_INODE_CSUM_NONZERO,
> +    "Inode checksum should not be set"
> +
>    end
> diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
> index ae7662e..1f08673 100644
> --- a/lib/ext2fs/ext2_fs.h
> +++ b/lib/ext2fs/ext2_fs.h
> @@ -362,7 +362,7 @@ struct ext2_inode_large {
>            __u16    l_i_file_acl_high;
>            __u16    l_i_uid_high;    /* these 2 fields    */
>            __u16    l_i_gid_high;    /* were reserved2[0] */
> -            __u32    l_i_reserved2;
> +            __u32    l_i_checksum;    /* crc32c(uuid+inum+inode) */
>        } linux2;
>        struct {
>            __u8    h_i_frag;    /* Fragment number */
> @@ -393,7 +393,7 @@ struct ext2_inode_large {
> #define i_gid_low    i_gid
> #define i_uid_high    osd2.linux2.l_i_uid_high
> #define i_gid_high    osd2.linux2.l_i_gid_high
> -#define i_reserved2    osd2.linux2.l_i_reserved2
> +#define i_checksum    osd2.linux2.l_i_checksum
> #else
> #if defined(__GNU__)
> 
> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
> index e571508..db8b28b 100644
> --- a/lib/ext2fs/ext2fs.h
> +++ b/lib/ext2fs/ext2fs.h
> @@ -892,6 +892,12 @@ extern __u32 crc32c_be(__u32 crc, unsigned char const *p, size_t len);
> extern __u32 crc32c_le(__u32 crc, unsigned char const *p, size_t len);
> 
> /* csum.c */
> +extern __u32 ext2fs_inode_csum(ext2_filsys fs, ext2_ino_t inum,
> +                  struct ext2_inode_large *inode);
> +extern void ext2fs_inode_csum_set(ext2_filsys fs, ext2_ino_t inum,
> +                  struct ext2_inode_large *inode);
> +extern int ext2fs_inode_csum_verify(ext2_filsys fs, ext2_ino_t inum,
> +                    struct ext2_inode_large *inode);
> extern void ext2fs_group_desc_csum_set(ext2_filsys fs, dgrp_t group);
> extern int ext2fs_group_desc_csum_verify(ext2_filsys fs, dgrp_t group);
> extern errcode_t ext2fs_set_gdt_csum(ext2_filsys fs);
> diff --git a/lib/ext2fs/inode.c b/lib/ext2fs/inode.c
> index 76893fd..0789505 100644
> --- a/lib/ext2fs/inode.c
> +++ b/lib/ext2fs/inode.c
> @@ -509,6 +509,12 @@ errcode_t ext2fs_get_next_inode_full(ext2_inode_scan scan, ext2_ino_t *ino,
>    if (EXT2_INODE_SIZE(scan->fs->super) == EXT2_GOOD_OLD_INODE_SIZE)
>        inode->i_extra_isize = 0;
> 
> +    /* Verify the inode checksum. */
> +    if (!(scan->fs->flags & EXT2_FLAG_IGNORE_CSUM_ERRORS) &&
> +        !ext2fs_inode_csum_verify(scan->fs, scan->current_inode, inode))
> +        return EXT2_ET_INODE_CSUM_INVALID;
> +
> +
>    scan->inodes_left--;
>    scan->current_inode++;
>    *ino = scan->current_inode;
> @@ -617,6 +623,10 @@ errcode_t ext2fs_read_inode_full(ext2_filsys fs, ext2_ino_t ino,
>                   (struct ext2_inode_large *) inode,
>                   0, bufsize);
> #endif
> +    /* Verify the inode checksum. */
> +    if (!(fs->flags & EXT2_FLAG_IGNORE_CSUM_ERRORS) &&
> +        !ext2fs_inode_csum_verify(fs, ino, inode))
> +        return EXT2_ET_INODE_CSUM_INVALID;
> 
>    /* Update the inode cache */
>    fs->icache->cache_last = (fs->icache->cache_last + 1) %
> @@ -685,6 +695,16 @@ errcode_t ext2fs_write_inode_full(ext2_filsys fs, ext2_ino_t ino,
>        w_inode = &temp_inode;
>    memset(w_inode, 0, length);
> 
> +    /*
> +     * If inode checksum enabled, ensure that we actually have the whole
> +     * inode in memory.
> +     */
> +    if (EXT2_INODE_SIZE(fs->super) > EXT2_GOOD_OLD_INODE_SIZE &&
> +        bufsize <= EXT2_GOOD_OLD_INODE_SIZE) {
> +        fprintf(stderr, "inode %d has a too-short buffer!\n", ino);
> +        abort();
> +    }
> +    ext2fs_inode_csum_set(fs, ino, inode);
> #ifdef WORDS_BIGENDIAN
>    ext2fs_swap_inode_full(fs, w_inode, inode, 1, bufsize);
> #else
> diff --git a/lib/ext2fs/swapfs.c b/lib/ext2fs/swapfs.c
> index 517f1d7..df604ba 100644
> --- a/lib/ext2fs/swapfs.c
> +++ b/lib/ext2fs/swapfs.c
> @@ -244,8 +244,7 @@ void ext2fs_swap_inode_full(ext2_filsys fs, struct ext2_inode_large *t,
>          ext2fs_swab16 (f->osd2.linux2.l_i_uid_high);
>        t->osd2.linux2.l_i_gid_high =
>          ext2fs_swab16 (f->osd2.linux2.l_i_gid_high);
> -        t->osd2.linux2.l_i_reserved2 =
> -            ext2fs_swab32(f->osd2.linux2.l_i_reserved2);
> +        t->i_checksum = ext2fs_swab32(f->i_checksum);
>        break;
>    case EXT2_OS_HURD:
>        t->osd1.hurd1.h_i_translator =
> @@ -279,6 +278,13 @@ void ext2fs_swap_inode_full(ext2_filsys fs, struct ext2_inode_large *t,
>        return;
>    }
> 
> +    t->i_ctime_extra = ext2fs_swab32(f->i_ctime_extra);
> +    t->i_mtime_extra = ext2fs_swab32(f->i_mtime_extra);
> +    t->i_atime_extra = ext2fs_swab32(f->i_atime_extra);
> +    t->i_crtime = ext2fs_swab32(f->i_crtime);
> +    t->i_crtime_extra = ext2fs_swab32(f->i_crtime_extra);
> +    t->i_version_hi = ext2fs_swab32(f->i_version_hi);
> +
>    i = EXT2_GOOD_OLD_INODE_SIZE + extra_isize + sizeof(__u32);
>    if (bufsize < (int) i)
>        return; /* no space for EA magic */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2011-09-04 17:59 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-01  0:35 [PATCH v1 00/37] e2fsprogs: Add metadata checksumming Darrick J. Wong
2011-09-01  0:35 ` [PATCH 01/37] e2fsprogs: Read and write full-sized inodes Darrick J. Wong
2011-09-03 18:05   ` Andreas Dilger
2011-09-04 14:04     ` Ted Ts'o
2011-09-04 17:40       ` Andreas Dilger
2011-09-14 16:39   ` Ted Ts'o
2011-09-15 20:25     ` Darrick J. Wong
2011-09-15 21:35       ` Andreas Dilger
2011-09-16  1:04         ` Darrick J. Wong
2011-09-18  1:52           ` Ted Ts'o
2011-09-01  0:35 ` [PATCH 02/37] libext2fs: Add metadata checksum flag Darrick J. Wong
2011-09-04  1:47   ` Andreas Dilger
2011-09-01  0:35 ` [PATCH 03/37] debugfs: Optionally ignore bad checksums Darrick J. Wong
2011-09-01  0:35 ` [PATCH 04/37] libext2fs: Add crc32c implementation for metadata checksumming Darrick J. Wong
2011-09-16  3:32   ` Ted Ts'o
2011-09-01  0:35 ` [PATCH 05/37] libext2fs: Implement a crc32c self-test Darrick J. Wong
2011-09-01  0:35 ` [PATCH 06/37] libext2fs: Add inode checksum support Darrick J. Wong
2011-09-04 17:59   ` Andreas Dilger [this message]
2011-09-05 18:59     ` Darrick J. Wong
2011-09-01  0:35 ` [PATCH 07/37] debugfs: Dump inode checksum when appropriate Darrick J. Wong
2011-09-01  0:36 ` [PATCH 08/37] tune2fs: Add inode checksum support Darrick J. Wong
2011-09-01  0:36 ` [PATCH 09/37] e2fsck: Verify and correct inode checksums Darrick J. Wong
2011-09-04 18:17   ` Andreas Dilger
2011-09-05 19:05     ` Darrick J. Wong
2011-09-01  0:36 ` [PATCH 10/37] mke2fs: Allow metadata checksums to be turned on at mkfs time Darrick J. Wong
2011-09-04 18:28   ` Andreas Dilger
2011-09-05 19:20     ` Darrick J. Wong
2011-09-06  1:54       ` Andreas Dilger
2011-09-06 17:13         ` Darrick J. Wong
2011-09-01  0:36 ` [PATCH 11/37] libext2fs: Create the inode bitmap checksum Darrick J. Wong
2011-09-14 17:02   ` Ted Ts'o
2011-09-14 19:31     ` Darrick J. Wong
2011-09-14 20:00       ` Andreas Dilger
2011-09-14 19:59     ` Andreas Dilger
2011-09-14 22:14       ` Ted Ts'o
2011-09-01  0:36 ` [PATCH 12/37] tune2fs: Rewrite inode bitmap checksums Darrick J. Wong
2011-09-01  0:36 ` [PATCH 13/37] dumpe2fs: Display inode bitmap checksum Darrick J. Wong
2011-09-04 18:30   ` Andreas Dilger
2011-09-05 19:20     ` Darrick J. Wong
2011-09-01  0:36 ` [PATCH 14/37] e2fsck: Verify " Darrick J. Wong
2011-09-01  0:36 ` [PATCH 15/37] libext2fs: Create the block " Darrick J. Wong
2011-09-01  0:36 ` [PATCH 16/37] dumpe2fs: Display " Darrick J. Wong
2011-09-01  0:37 ` [PATCH 17/37] e2fsck: Verify " Darrick J. Wong
2011-09-01  0:37 ` [PATCH 18/37] e2fsck: Don't verify bitmap checksums Darrick J. Wong
2011-09-01  0:37 ` [PATCH 19/37] tune2fs: Rewrite block " Darrick J. Wong
2011-09-01  0:37 ` [PATCH 20/37] libext2fs: Verify and calculate extent tree block checksums Darrick J. Wong
2011-09-01  0:37 ` [PATCH 21/37] tune2fs: Enable extent tree checksums Darrick J. Wong
2011-09-01  0:37 ` [PATCH 22/37] libext2fs: Introduce dx_tail and dir_entry_tail Darrick J. Wong
2011-09-01  0:37 ` [PATCH 23/37] debugfs: Print htree internal node checksums Darrick J. Wong
2011-09-01  0:37 ` [PATCH 24/37] libext2fs: Add dx_root/dx_node checksum calculation and verification helpers Darrick J. Wong
2011-09-01  0:37 ` [PATCH 25/37] e2fsck: Verify htree root/node checksums Darrick J. Wong
2011-09-01  0:37 ` [PATCH 26/37] libext2fs: Introduce dir_entry_tail to provide checksums for directory leaf nodes Darrick J. Wong
2011-09-01  0:38 ` [PATCH 27/37] e2fsck: Check directory leaf block checksums Darrick J. Wong
2011-09-01  0:38 ` [PATCH 28/37] tune2fs: Rebuild and checksum directories when toggling metadata_csum or changing UUID Darrick J. Wong
2011-09-01  0:38 ` [PATCH 29/37] libext2fs: Verify and calculate extended attribute block checksums Darrick J. Wong
2011-09-01  0:38 ` [PATCH 30/37] e2fsck: Check " Darrick J. Wong
2011-09-01  0:38 ` [PATCH 32/37] libext2fs: Extend inode checksum to cover the EA block Darrick J. Wong
2011-09-14 16:48   ` Ted Ts'o
2011-09-01  0:38 ` [PATCH 33/37] tune2fs: Rewrite extended attribute block checksums Darrick J. Wong
2011-09-01  0:38 ` [PATCH 34/37] libext2fs: Calculate and verify superblock checksums Darrick J. Wong
2011-09-01  0:38 ` [PATCH 35/37] e2fsck: Handle superblock checksum errors gracefully Darrick J. Wong
2011-09-01  0:39 ` [PATCH 36/37] e2p: Print superblock checksum in list_super Darrick J. Wong
2011-09-01  0:39 ` [PATCH 37/37] e2fsck: Support CRC32c checksum in journal commit blocks Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=13B69EAD-7ABF-437C-95A4-3FA80ED98562@gmail.com \
    --to=aedilger@gmail.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=amir73il@gmail.com \
    --cc=andi@firstfloor.org \
    --cc=cmm@us.ibm.com \
    --cc=colyli@gmail.com \
    --cc=djwong@us.ibm.com \
    --cc=jlbec@evilplan.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=sunil.mushran@oracle.com \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.