Do not use safe_malloc for LUKS header backup.

The content of LUKS header is not a key material, no need
to lock memory for possibly big header and big memory area locks.

Just ensure we wipe buffer before release of memory.
This commit is contained in:
Milan Broz
2022-08-16 15:34:54 +02:00
parent db65a5ceac
commit 21d87a246e
3 changed files with 23 additions and 19 deletions

View File

@@ -232,7 +232,7 @@ int LUKS_hdr_backup(const char *backup_file, struct crypt_device *ctx)
hdr_size = LUKS_device_sectors(&hdr) << SECTOR_SHIFT; hdr_size = LUKS_device_sectors(&hdr) << SECTOR_SHIFT;
buffer_size = size_round_up(hdr_size, crypt_getpagesize()); buffer_size = size_round_up(hdr_size, crypt_getpagesize());
buffer = crypt_safe_alloc(buffer_size); buffer = malloc(buffer_size);
if (!buffer || hdr_size < LUKS_ALIGN_KEYSLOTS || hdr_size > buffer_size) { if (!buffer || hdr_size < LUKS_ALIGN_KEYSLOTS || hdr_size > buffer_size) {
r = -ENOMEM; r = -ENOMEM;
goto out; goto out;
@@ -280,7 +280,8 @@ int LUKS_hdr_backup(const char *backup_file, struct crypt_device *ctx)
r = 0; r = 0;
out: out:
crypt_safe_memzero(&hdr, sizeof(hdr)); crypt_safe_memzero(&hdr, sizeof(hdr));
crypt_safe_free(buffer); crypt_safe_memzero(buffer, buffer_size);
free(buffer);
return r; return r;
} }
@@ -308,7 +309,7 @@ int LUKS_hdr_restore(
goto out; goto out;
} }
buffer = crypt_safe_alloc(buffer_size); buffer = malloc(buffer_size);
if (!buffer) { if (!buffer) {
r = -ENOMEM; r = -ENOMEM;
goto out; goto out;
@@ -379,7 +380,8 @@ int LUKS_hdr_restore(
r = LUKS_read_phdr(hdr, 1, 0, ctx); r = LUKS_read_phdr(hdr, 1, 0, ctx);
out: out:
device_sync(ctx, device); device_sync(ctx, device);
crypt_safe_free(buffer); crypt_safe_memzero(buffer, buffer_size);
free(buffer);
return r; return r;
} }

View File

@@ -1246,7 +1246,7 @@ int LUKS2_hdr_backup(struct crypt_device *cd, struct luks2_hdr *hdr,
hdr_size = LUKS2_hdr_and_areas_size(hdr); hdr_size = LUKS2_hdr_and_areas_size(hdr);
buffer_size = size_round_up(hdr_size, crypt_getpagesize()); buffer_size = size_round_up(hdr_size, crypt_getpagesize());
buffer = crypt_safe_alloc(buffer_size); buffer = malloc(buffer_size);
if (!buffer) if (!buffer)
return -ENOMEM; return -ENOMEM;
@@ -1257,23 +1257,22 @@ int LUKS2_hdr_backup(struct crypt_device *cd, struct luks2_hdr *hdr,
if (r) { if (r) {
log_err(cd, _("Failed to acquire read lock on device %s."), log_err(cd, _("Failed to acquire read lock on device %s."),
device_path(crypt_metadata_device(cd))); device_path(crypt_metadata_device(cd)));
crypt_safe_free(buffer); goto out;
return r;
} }
devfd = device_open_locked(cd, device, O_RDONLY); devfd = device_open_locked(cd, device, O_RDONLY);
if (devfd < 0) { if (devfd < 0) {
device_read_unlock(cd, device); device_read_unlock(cd, device);
log_err(cd, _("Device %s is not a valid LUKS device."), device_path(device)); log_err(cd, _("Device %s is not a valid LUKS device."), device_path(device));
crypt_safe_free(buffer); r = (devfd == -1) ? -EINVAL : devfd;
return devfd == -1 ? -EINVAL : devfd; goto out;
} }
if (read_lseek_blockwise(devfd, device_block_size(cd, device), if (read_lseek_blockwise(devfd, device_block_size(cd, device),
device_alignment(device), buffer, hdr_size, 0) < hdr_size) { device_alignment(device), buffer, hdr_size, 0) < hdr_size) {
device_read_unlock(cd, device); device_read_unlock(cd, device);
crypt_safe_free(buffer); r = -EIO;
return -EIO; goto out;
} }
device_read_unlock(cd, device); device_read_unlock(cd, device);
@@ -1284,8 +1283,8 @@ int LUKS2_hdr_backup(struct crypt_device *cd, struct luks2_hdr *hdr,
log_err(cd, _("Requested header backup file %s already exists."), backup_file); log_err(cd, _("Requested header backup file %s already exists."), backup_file);
else else
log_err(cd, _("Cannot create header backup file %s."), backup_file); log_err(cd, _("Cannot create header backup file %s."), backup_file);
crypt_safe_free(buffer); r = -EINVAL;
return -EINVAL; goto out;
} }
ret = write_buffer(fd, buffer, buffer_size); ret = write_buffer(fd, buffer, buffer_size);
close(fd); close(fd);
@@ -1294,8 +1293,9 @@ int LUKS2_hdr_backup(struct crypt_device *cd, struct luks2_hdr *hdr,
r = -EIO; r = -EIO;
} else } else
r = 0; r = 0;
out:
crypt_safe_free(buffer); crypt_safe_memzero(buffer, buffer_size);
free(buffer);
return r; return r;
} }
@@ -1340,7 +1340,7 @@ int LUKS2_hdr_restore(struct crypt_device *cd, struct luks2_hdr *hdr,
} }
buffer_size = LUKS2_hdr_and_areas_size(&hdr_file); buffer_size = LUKS2_hdr_and_areas_size(&hdr_file);
buffer = crypt_safe_alloc(buffer_size); buffer = malloc(buffer_size);
if (!buffer) { if (!buffer) {
r = -ENOMEM; r = -ENOMEM;
goto out; goto out;
@@ -1440,10 +1440,9 @@ out:
LUKS2_hdr_free(cd, &tmp_hdr); LUKS2_hdr_free(cd, &tmp_hdr);
crypt_safe_memzero(&hdr_file, sizeof(hdr_file)); crypt_safe_memzero(&hdr_file, sizeof(hdr_file));
crypt_safe_memzero(&tmp_hdr, sizeof(tmp_hdr)); crypt_safe_memzero(&tmp_hdr, sizeof(tmp_hdr));
crypt_safe_free(buffer); crypt_safe_memzero(buffer, buffer_size);
free(buffer);
device_sync(cd, device); device_sync(cd, device);
return r; return r;
} }

View File

@@ -38,6 +38,9 @@ struct safe_allocation {
*/ */
void crypt_safe_memzero(void *data, size_t size) void crypt_safe_memzero(void *data, size_t size)
{ {
if (!data)
return;
#ifdef HAVE_EXPLICIT_BZERO #ifdef HAVE_EXPLICIT_BZERO
explicit_bzero(data, size); explicit_bzero(data, size);
#else #else