[PATCH 1/8] lib/blkid/list.h: Fix undefined behavior in list_entry() macro

[PATCH 1/8] lib/blkid/list.h: Fix undefined behavior in list_entry() macro

[Sami Liedes]

Update list_entry() macro, which is basically the same as the
container_of() macro in the kernel, to use offsetof() to fix undefined
behavior.

Caught using clang -fsanitize=undefined.

Signed-off-by: Sami Liedes <sami.liedes@iki.fi>
---
 lib/blkid/list.h |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/blkid/list.h b/lib/blkid/list.h
index c1cbfec..d15228e 100644
--- a/lib/blkid/list.h
+++ b/lib/blkid/list.h
@@ -148,8 +148,9 @@ _INLINE_ void list_splice(struct list_head *list, struct list_head *head)
  * @type:	the type of the struct this is embedded in.
  * @member:	the name of the list_struct within the struct.
  */
-#define list_entry(ptr, type, member) \
-	((type *)((char *)(ptr)-(unsigned long)(&((type *)0)->member)))
+#define list_entry(ptr, type, member) ({		       \
+	const typeof( ((type *)0)->member ) *__mptr = (ptr);   \
+	(type *)( (char *)__mptr - offsetof(type,member) );})
 
 /**
  * list_for_each - iterate over elements in a list
-- 
1.7.10.4

[PATCH 2/8] lib/ext2fs/rbtree.h: Fix container_of() undefined behavior

[Sami Liedes]

The code before the macro definition #undefs and redefines offsetof().
Unfortunately the new definition actually causes undefined behavior.

The code checks for __compiler_offsetof() before redefining. However
I'm not sure where it is supposed to be defined.

Just enclose the redefinition in #ifndef __GNUC__ for now.

Caught using clang -fsanitize=undefined.

Signed-off-by: Sami Liedes <sami.liedes@iki.fi>
---
 lib/ext2fs/rbtree.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/ext2fs/rbtree.h b/lib/ext2fs/rbtree.h
index 16defb5..088c352 100644
--- a/lib/ext2fs/rbtree.h
+++ b/lib/ext2fs/rbtree.h
@@ -96,12 +96,14 @@ static inline struct page * rb_insert_page_cache(struct inode * inode,
 
 #include <stdlib.h>
 
+#ifndef __GNUC__
 #undef offsetof
 #ifdef __compiler_offsetof
 #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)
 #else
 #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 #endif
+#endif
 
 #define container_of(ptr, type, member) ({			\
 	const __typeof__( ((type *)0)->member ) *__mptr = (ptr);	\
-- 
1.7.10.4

[PATCH 3/8] e2fsck/pass1.c: Fix undefined behavior in check_blocks()

[Sami Liedes]

The offending code is this:

   pb.max_blocks = 1 << (31 - fs->super->s_log_block_size);

While pb.max_blocks is of type blk64_t, the intermediate result of the
expression '1 << 31' is int. However 1 << 31 does not fit in a 32-bit
signed int, causing undefined behavior.

Caught using clang -fsanitize=undefined.

Signed-off-by: Sami Liedes <sami.liedes@iki.fi>
---
 e2fsck/pass1.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index a4bd956..9cd8832 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -2095,7 +2095,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 = ((blk64_t)1) << (31 - fs->super->s_log_block_size);
 	pb.inode = inode;
 	pb.pctx = pctx;
 	pb.ctx = ctx;
-- 
1.7.10.4

[PATCH 4/8] lib/ext2fs/block.c: Fix undefined behavior in block_iterate_tind()

[Sami Liedes]

The intermediate result of limit*limit*limit is too big to fit in a
32-bit integer, causing undefined behavior. Fix by casting one of the
limits to the result type of e2_blkcnt_t.

Caught using clang -fsanitize=undefined.

Signed-off-by: Sami Liedes <sami.liedes@iki.fi>
---
 lib/ext2fs/block.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ext2fs/block.c b/lib/ext2fs/block.c
index 68dcb03..dd3911d 100644
--- 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 += ((e2_blkcnt_t)limit)*limit*limit;
 		return ret;
 	}
 	if (*tind_block >= ext2fs_blocks_count(ctx->fs->super) ||
-- 
1.7.10.4

[PATCH 5/8] e2fsck/revoke.c: Fix undefined behavior in hash()

[Sami Liedes]

There's a shift by (hash_shift - 12) in hash(), but hash() is called
with hash_shift=10, resulting in a negative shift and thus undefined
behavior.

A simple and stupid minimal fix is to just change the subtracted
amount to 10.

Caught by clang -fsanitize=undefined.

Signed-off-by: Sami Liedes <sami.liedes@iki.fi>
---
 e2fsck/revoke.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/e2fsck/revoke.c b/e2fsck/revoke.c
index 38c265e..dddef4d 100644
--- a/e2fsck/revoke.c
+++ b/e2fsck/revoke.c
@@ -115,7 +115,7 @@ static inline int hash(journal_t *journal, unsigned long block)
 
 	return ((block << (hash_shift - 6)) ^
 		(block >> 13) ^
-		(block << (hash_shift - 12))) & (table->hash_size - 1);
+		(block << (hash_shift - 10))) & (table->hash_size - 1);
 }
 
 static int insert_revoke_hash(journal_t *journal, unsigned long blocknr,
-- 
1.7.10.4

[PATCH 6/8] lib/ext2fs/kernel-list.h: Fix undefined behavior in list_entry() macro

[Sami Liedes]

Fix the macro, which is essentially a copy of the container_of() in
the kernel, to use offsetof instead of a null pointer dereference.

Caught by clang -fsanitize=undefined.

Signed-off-by: Sami Liedes <sami.liedes@iki.fi>
---
 lib/ext2fs/kernel-list.h |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/ext2fs/kernel-list.h b/lib/ext2fs/kernel-list.h
index e07d06b..85ae213 100644
--- a/lib/ext2fs/kernel-list.h
+++ b/lib/ext2fs/kernel-list.h
@@ -103,8 +103,10 @@ static __inline__ void list_splice(struct list_head *list, struct list_head *hea
 	}
 }
 
-#define list_entry(ptr, type, member) \
-	((type *)((char *)(ptr)-(unsigned long)(&((type *)0)->member)))
+#define list_entry(ptr, type, member) ({                      \
+       const typeof( ((type *)0)->member ) *__mptr = (ptr);   \
+       (type *)( (char *)__mptr - offsetof(type,member) );})
+
 
 #define list_for_each(pos, head) \
         for (pos = (head)->next; pos != (head); pos = pos->next)
-- 
1.7.10.4

[PATCH 7/8] lib/ext2fs/qcow2.h: Fix #defined 1<<63 values to be unsigned.

[Sami Liedes]

QCOW_OFLAG_COPIED in qcow2.h and e2image.c was defined as a signed
value, which however does not fit in a signed 64-bit integer.

Caught by clang -fsanitize=undefined.

Signed-off-by: Sami Liedes <sami.liedes@iki.fi>
---
 lib/ext2fs/qcow2.h |    4 ++--
 misc/e2image.c     |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/ext2fs/qcow2.h b/lib/ext2fs/qcow2.h
index 81e0ec9..b997653 100644
--- a/lib/ext2fs/qcow2.h
+++ b/lib/ext2fs/qcow2.h
@@ -30,8 +30,8 @@
 
 #define QCOW_MAGIC (('Q' << 24) | ('F' << 16) | ('I' << 8) | 0xfb)
 #define QCOW_VERSION		2
-#define QCOW_OFLAG_COPIED	(1LL << 63)
-#define QCOW_OFLAG_COMPRESSED	(1LL << 62)
+#define QCOW_OFLAG_COPIED	(1LLU << 63)
+#define QCOW_OFLAG_COMPRESSED	(1LLU << 62)
 
 #define QCOW_COMPRESSED		1
 #define QCOW_ENCRYPTED		2
diff --git a/misc/e2image.c b/misc/e2image.c
index e6ea52a..36769c1 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -47,7 +47,7 @@ extern int optind;
 #include "../version.h"
 #include "nls-enable.h"
 
-#define QCOW_OFLAG_COPIED     (1LL << 63)
+#define QCOW_OFLAG_COPIED     (1LLU << 63)
 
 
 const char * program_name = "e2image";
-- 
1.7.10.4

[PATCH 8/8] e2fsck/jfs_user.h: Fix b_data alignment in struct buffer_head

[Sami Liedes]

buffer_head.b_data needs to be 8-byte aligned to prevent an unaligned
access via a 64-bit pointer in e.g. scan_revoke_records().

Caught using clang -fsanitize=undefined.

Signed-off-by: Sami Liedes <sami.liedes@iki.fi>
---
 e2fsck/jfs_user.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/e2fsck/jfs_user.h b/e2fsck/jfs_user.h
index 3cccd3f..fdaf1b2 100644
--- a/e2fsck/jfs_user.h
+++ b/e2fsck/jfs_user.h
@@ -22,7 +22,7 @@ struct buffer_head {
 	int	 	b_dirty;
 	int	 	b_uptodate;
 	int	 	b_err;
-	char		b_data[1024];
+	char		b_data[1024]  __attribute__ ((aligned (8)));
 };
 
 struct inode {
-- 
1.7.10.4