On Fri, Nov 18, 2016 at 01:38:39PM -0500, Theodore Ts'o wrote: > @@ -3567,7 +3567,15 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) > if (blocksize < EXT4_MIN_BLOCK_SIZE || > blocksize > EXT4_MAX_BLOCK_SIZE) { > ext4_msg(sb, KERN_ERR, > - "Unsupported filesystem blocksize %d", blocksize); > + "Unsupported filesystem blocksize %d (%d log_block_size)", > + blocksize, le32_to_cpu(es->s_log_block_size)); > + goto failed_mount; > + } > + if (le32_to_cpu(es->s_log_block_size) > > + (EXT4_MAX_BLOCK_LOG_SIZE - EXT4_MIN_BLOCK_LOG_SIZE)) { > + ext4_msg(sb, KERN_ERR, > + "Invalid log block size: %u", > + le32_to_cpu(es->s_log_block_size)); > goto failed_mount; > } > This isn't validating s_log_block_size until after it's already been used in a shift, which means the code can have undefined behavior (shift by a value too large). Would it make sense to do something like the following instead? Similarly for the cluster size case. blocksize = BLOCK_SIZE << min_t(u32, le32_to_cpu(es->s_log_block_size), 20); if (blocksize < EXT4_MIN_BLOCK_SIZE || blocksize > EXT4_MAX_BLOCK_SIZE) { ext4_msg(sb, KERN_ERR, "Unsupported filesystem blocksize %d (%u bits)", blocksize, le32_to_cpu(es->s_log_block_size)); goto failed_mount; } -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html