Fix warnings found using UBSAN

Compiling with -fsanitize=undefined -fsanitize=address causes some
warnings of C code that has undefined behavior according to the C
standard bugs.  None of the warnings should cause e2fsprogs
malfunction given a sane compiler running on architectures that Linux
can support.  Still, it's better to clean up to code than not.

To fix up a complaint of a negative shift in hash function, update the
very dated hash we had been using for the revoke table with the
current generic hash used by the kernel.

Other fixes are fairly self-explanatory.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/e2fsck/jfs_user.h b/e2fsck/jfs_user.h
index 75877f3..d30b728 100644 (file)
--- a/e2fsck/jfs_user.h
+++ b/e2fsck/jfs_user.h
@@ -137,6 +137,32 @@ _INLINE_ void do_cache_destroy(lkmem_cache_t *cache)
        free(cache);
 }
 
+/* generic hashing taken from the Linux kernel */
+#define GOLDEN_RATIO_32 0x61C88647
+#define GOLDEN_RATIO_64 0x61C8864680B583EBull
+
+_INLINE_ __u32 __hash_32(__u32 val)
+{
+       return val * GOLDEN_RATIO_32;
+}
+
+_INLINE_ __u32 hash_32(__u32 val, unsigned int bits)
+{
+       /* High bits are more random, so use them. */
+       return __hash_32(val) >> (32 - bits);
+}
+
+_INLINE_ __u32 hash_64(__u64 val, unsigned int bits)
+{
+       if (sizeof(long) >= 8) {
+               /* 64x64-bit multiply is efficient on all 64-bit processors */
+               return val * GOLDEN_RATIO_64 >> (64 - bits);
+       } else {
+               /* Hash 64 bits using only 32x32-bit multiply. */
+               return hash_32((__u32)val ^ __hash_32(val >> 32), bits);
+       }
+}
+
 #undef _INLINE_
 #endif
 

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 7983ffd..160e677 100644 (file)
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -449,7 +449,7 @@ fix:
 }
 
 static int check_inode_extra_negative_epoch(__u32 xtime, __u32 extra) {
-       return (xtime & (1 << 31)) != 0 &&
+       return (xtime & (1U << 31)) != 0 &&
                (extra & EXT4_EPOCH_MASK) == EXT4_EPOCH_MASK;
 }
 
@@ -3072,7 +3072,7 @@ static void check_blocks(e2fsck_t ctx, struct problem_context *pctx,
        pb.previous_block = 0;
        pb.is_dir = LINUX_S_ISDIR(inode->i_mode);
        pb.is_reg = LINUX_S_ISREG(inode->i_mode);
-       pb.max_blocks = 1 << (31 - fs->super->s_log_block_size);
+       pb.max_blocks = 1U << (31 - fs->super->s_log_block_size);
        pb.inode = inode;
        pb.pctx = pctx;
        pb.ctx = ctx;

diff --git a/e2fsck/recovery.c b/e2fsck/recovery.c
index abf12c7..4c457b3 100644 (file)
--- a/e2fsck/recovery.c
+++ b/e2fsck/recovery.c
@@ -124,6 +124,27 @@ failed:
 
 #endif /* __KERNEL__ */
 
+static inline __u32 get_be32(__be32 *p)
+{
+       unsigned char *cp = (unsigned char *) p;
+       __u32 ret;
+
+       ret = *cp++;
+       ret = (ret << 8) + *cp++;
+       ret = (ret << 8) + *cp++;
+       ret = (ret << 8) + *cp++;
+       return ret;
+}
+
+static inline __u16 get_be16(__be16 *p)
+{
+       unsigned char *cp = (unsigned char *) p;
+       __u16 ret;
+
+       ret = *cp++;
+       ret = (ret << 8) + *cp++;
+       return ret;
+}
 
 /*
  * Read a block from the journal
@@ -215,10 +236,10 @@ static int count_tags(journal_t *journal, struct buffer_head *bh)
 
                nr++;
                tagp += tag_bytes;
-               if (!(tag->t_flags & ext2fs_cpu_to_be16(JFS_FLAG_SAME_UUID)))
+               if (!(get_be16(&tag->t_flags) & JFS_FLAG_SAME_UUID))
                        tagp += 16;
 
-               if (tag->t_flags & ext2fs_cpu_to_be16(JFS_FLAG_LAST_TAG))
+               if (get_be16(&tag->t_flags) & JFS_FLAG_LAST_TAG)
                        break;
        }
 
@@ -338,18 +359,6 @@ int journal_skip_recovery(journal_t *journal)
        return err;
 }
 
-static inline __u32 get_be32(__be32 *p)
-{
-       unsigned char *cp = (unsigned char *) p;
-       __u32 ret;
-
-       ret = *cp++;
-       ret = (ret << 8) + *cp++;
-       ret = (ret << 8) + *cp++;
-       ret = (ret << 8) + *cp++;
-       return ret;
-}
-
 static inline unsigned long long read_tag_block(journal_t *journal,
                                                journal_block_tag_t *tag)
 {
@@ -424,9 +433,9 @@ static int jbd2_block_tag_csum_verify(journal_t *j, journal_block_tag_t *tag,
        csum32 = jbd2_chksum(j, csum32, buf, j->j_blocksize);
 
        if (jfs_has_feature_csum3(j))
-               return tag3->t_checksum == ext2fs_cpu_to_be32(csum32);
+               return get_be32(&tag3->t_checksum) == csum32;
 
-       return tag->t_checksum == ext2fs_cpu_to_be16(csum32);
+       return get_be16(&tag->t_checksum) == (csum32 & 0xFFFF);
 }
 
 static int do_one_pass(journal_t *journal,
@@ -574,7 +583,7 @@ static int do_one_pass(journal_t *journal,
                                unsigned long io_block;
 
                                tag = (journal_block_tag_t *) tagp;
-                               flags = ext2fs_be16_to_cpu(tag->t_flags);
+                               flags = get_be16(&tag->t_flags);
 
                                io_block = next_log_block++;
                                wrap(journal, next_log_block);

diff --git a/e2fsck/revoke.c b/e2fsck/revoke.c
index 0543099..64b897a 100644 (file)
--- a/e2fsck/revoke.c
+++ b/e2fsck/revoke.c
@@ -134,12 +134,8 @@ static void flush_descriptor(journal_t *, struct buffer_head *, int, int);
 static inline int hash(journal_t *journal, unsigned long long block)
 {
        struct jbd2_revoke_table_s *table = journal->j_revoke;
-       int hash_shift = table->hash_shift;
-       int hash = (int)block ^ (int)((block >> 31) >> 1);
 
-       return ((hash << (hash_shift - 6)) ^
-               (hash >> 13) ^
-               (hash << (hash_shift - 12))) & (table->hash_size - 1);
+       return (hash_64(block, table->hash_shift));
 }
 
 static int insert_revoke_hash(journal_t *journal, unsigned long long blocknr,

diff --git a/lib/ext2fs/block.c b/lib/ext2fs/block.c
index 601129d..06eed6e 100644 (file)
--- a/lib/ext2fs/block.c
+++ b/lib/ext2fs/block.c
@@ -251,7 +251,7 @@ static int block_iterate_tind(blk_t *tind_block, blk_t ref_block,
        }
        check_for_ro_violation_return(ctx, ret);
        if (!*tind_block || (ret & BLOCK_ABORT)) {
-               ctx->bcount += limit*limit*limit;
+               ctx->bcount += ((unsigned long long) limit)*limit*limit;
                return ret;
        }
        if (*tind_block >= ext2fs_blocks_count(ctx->fs->super) ||

diff --git a/lib/ext2fs/inode.c b/lib/ext2fs/inode.c
index ba7ad2c..8dc9ac0 100644 (file)
--- a/lib/ext2fs/inode.c
+++ b/lib/ext2fs/inode.c
@@ -630,7 +630,8 @@ errcode_t ext2fs_get_next_inode_full(ext2_inode_scan scan, ext2_ino_t *ino,
         * need to read in more blocks.
         */
        if (scan->bytes_left < scan->inode_size) {
-               memcpy(scan->temp_buffer, scan->ptr, scan->bytes_left);
+               if (scan->bytes_left)
+                       memcpy(scan->temp_buffer, scan->ptr, scan->bytes_left);
                extra_bytes = scan->bytes_left;
 
                retval = get_next_blocks(scan);