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 75877f33dfce964817ddeb3a199eb7fb8f05742f..d30b728d1e8525210964d377f8612ee0abe6348c 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 7983ffd15befa04ad459c6735803bfe848d80add..160e677ab212b6eea1daf2bc03bd4333974ea808 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 abf12c71ef2e65045442157656896a8a6ab32b63..4c457b39359811fac2ead35d28589fa1df7f8c80 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 0543099725af45c898c3c53a6356ac7edc152778..64b897a7a4171ae12d983bcf484b42809435e18b 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 601129d48e9cd2238f64685e6b89d8d23873d9ac..06eed6e0301c3a44f928018c98ff1052da876990 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 ba7ad2c9c4d46ac91abe0483d074600db1bb1e90..8dc9ac0d9e71ed1f734de02eb81b034239e07124 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);