Skip to content

Commit

Permalink
src/verity_hash: Get rid of off_t integers and use uint64_t.
Browse files Browse the repository at this point in the history 
This fixes lgtm checker error:

|                 if (hash_level_size)
|                         hash_level_size[i] = s;
|                 if ((*hash_position + s) < *hash_position ||
| Testing for signed overflow may produce undefined results.
| This alert was introduced in ae1b1149 months ago
|                     (*hash_position + s) < 0)
|                         return -EINVAL;

by applying changes done in upstream commit 36fd8d6b.

Original commit 36fd8d6b from cryptsetup:

| Author:     Milan Broz <[email protected]>
| AuthorDate: Sat Feb 13 20:43:33 2021 +0100
|
|     Get rid of off_t integers and use uint64_t.
|
|     Also move uint64 multiplication overflow check to internal library.

Unlike the original commit, we rename but do not move uint64_mult_overflow().

Signed-off-by: Enrico Joerns <[email protected]>
  • Loading branch information
@ejoerns
ejoerns committed Aug 19, 2021
1 parent 348c290 commit 790abcf
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 37 deletions.
4 changes: 2 additions & 2 deletions include/verity_hash.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
int verity_create_or_verify_hash(
int verify,
int fd,
off_t data_blocks,
off_t *combined_blocks,
uint64_t data_blocks,
uint64_t *combined_blocks,
uint8_t *root_hash,
const uint8_t *salt);
4 changes: 2 additions & 2 deletions src/bundle.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ static gboolean sign_bundle(const gchar *bundlename, RaucManifest *manifest, GEr
int bundlefd = g_file_descriptor_based_get_fd(G_FILE_DESCRIPTOR_BASED(bundleoutstream));
guint8 salt[32] = {0};
guint8 hash[32] = {0};
off_t combined_size = 0;
uint64_t combined_size = 0;
guint64 verity_size = 0;

g_print("Creating bundle in 'verity' format\n");
Expand Down Expand Up @@ -545,7 +545,7 @@ static gboolean sign_bundle(const gchar *bundlename, RaucManifest *manifest, GEr
return FALSE;
}
/* for a squashfs <= 4096 bytes, we don't have a hash table */
g_assert(combined_size*4096 > (off_t)offset);
g_assert(combined_size*4096 > (uint64_t)offset);
verity_size = combined_size*4096 - offset;
g_assert(verity_size % 4096 == 0);

Expand Down
54 changes: 24 additions & 30 deletions src/verity_hash.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,20 @@ static int verify_hash_block(
return r;
}

static int mult_overflow(off_t *u, off_t b, size_t size)
static gboolean uint64_mult_overflow(uint64_t *u, uint64_t b, size_t size)
{
*u = (uint64_t)b * size;
if ((off_t)(*u / size) != b || (off_t)*u < 0)
return 1;
return 0;
if ((uint64_t)(*u / size) != b)
return TRUE;
return FALSE;
}

static int hash_levels(
off_t data_file_blocks, off_t *hash_position, int *levels,
off_t *hash_level_block, off_t *hash_level_size)
uint64_t data_file_blocks, uint64_t *hash_position, int *levels,
uint64_t *hash_level_block, uint64_t *hash_level_size)
{
size_t hash_per_block_bits;
off_t s, s_shift;
uint64_t s, s_shift;
int i;

if (!digest_size)
Expand All @@ -158,11 +158,10 @@ static int hash_levels(
s_shift = (i + 1) * hash_per_block_bits;
if (s_shift > 63)
return -EINVAL;
s = (data_file_blocks + ((off_t)1 << s_shift) - 1) >> ((i + 1) * hash_per_block_bits);
s = (data_file_blocks + ((uint64_t)1 << s_shift) - 1) >> ((i + 1) * hash_per_block_bits);
if (hash_level_size)
hash_level_size[i] = s;
if ((*hash_position + s) < *hash_position ||
(*hash_position + s) < 0)
if ((*hash_position + s) < *hash_position)
return -EINVAL;
*hash_position += s;
}
Expand All @@ -171,9 +170,9 @@ static int hash_levels(
}

static int create_or_verify(FILE *rd, FILE *wr,
off_t data_block,
off_t hash_block,
off_t blocks,
uint64_t data_block,
uint64_t hash_block,
uint64_t blocks,
int verify,
uint8_t *calculated_digest,
const uint8_t *salt)
Expand All @@ -183,14 +182,14 @@ static int create_or_verify(FILE *rd, FILE *wr,
uint8_t read_digest[digest_size];
size_t hash_per_block = 1 << get_bits_down(hash_block_size / digest_size);
size_t digest_size_full = 1 << get_bits_up(digest_size);
off_t blocks_to_write = (blocks + hash_per_block - 1) / hash_per_block;
off_t seek_rd, seek_wr;
uint64_t blocks_to_write = (blocks + hash_per_block - 1) / hash_per_block;
uint64_t seek_rd, seek_wr;
size_t left_bytes;
unsigned i;
int r;

if (mult_overflow(&seek_rd, data_block, data_block_size) ||
mult_overflow(&seek_wr, hash_block, hash_block_size)) {
if (uint64_mult_overflow(&seek_rd, data_block, data_block_size) ||
uint64_mult_overflow(&seek_wr, hash_block, hash_block_size)) {
g_message("Device offset overflow.");
return -EINVAL;
}
Expand Down Expand Up @@ -273,31 +272,26 @@ static int create_or_verify(FILE *rd, FILE *wr,
int verity_create_or_verify_hash(
int verify,
int fd,
off_t data_blocks,
off_t *combined_blocks,
uint64_t data_blocks,
uint64_t *combined_blocks,
uint8_t *root_hash,
const uint8_t *salt)
{
g_autofree gchar *file = NULL;
off_t hash_position = data_blocks;
uint64_t hash_position = data_blocks;
uint8_t calculated_digest[digest_size];
FILE *data_file = NULL;
FILE *hash_file = NULL, *hash_file_2;
off_t hash_level_block[VERITY_MAX_LEVELS];
off_t hash_level_size[VERITY_MAX_LEVELS];
off_t data_device_size = 0, hash_device_size = 0;
uint64_t hash_level_block[VERITY_MAX_LEVELS];
uint64_t hash_level_size[VERITY_MAX_LEVELS];
uint64_t data_device_size = 0, hash_device_size = 0;
int levels, i, r;

g_debug("Hash %s %s, data blocks %" PRIu64 ".",
verify ? "verification" : "creation", "SHA256",
data_blocks);

if (data_blocks < 0 || hash_position < 0) {
g_message("Invalid size parameters for verity device.");
return -EINVAL;
}

if (mult_overflow(&data_device_size, data_blocks, data_block_size)) {
if (uint64_mult_overflow(&data_device_size, data_blocks, data_block_size)) {
g_message("Device offset overflow.");
return -EINVAL;
}
Expand All @@ -310,7 +304,7 @@ int verity_create_or_verify_hash(

g_debug("Using %d hash levels.", levels);

if (mult_overflow(&hash_device_size, hash_position, hash_block_size)) {
if (uint64_mult_overflow(&hash_device_size, hash_position, hash_block_size)) {
g_message("Device offset overflow.");
return -EINVAL;
}
Expand Down
6 changes: 3 additions & 3 deletions test/dm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ typedef struct {
} DMFixture;

typedef struct {
off_t data_size;
off_t combined_size;
uint64_t data_size;
uint64_t combined_size;
} DMData;

static void dm_fixture_set_up(DMFixture *fixture,
Expand Down Expand Up @@ -211,7 +211,7 @@ static void verity_hash_create(DMFixture *fixture,
g_autofree gchar *root_hash_hex = NULL;
g_autofree guint8 *salt = random_bytes(32, 0xd6368505);
g_autofree gchar *salt_hex = r_hex_encode(salt, 32);
off_t combined_size;
uint64_t combined_size;
int dmfd = -1;

/* needs to run as root */
Expand Down

0 comments on commit 790abcf

Please sign in to comment.