Fix popt string related memory leaks.

All POPT_ARG_STRING pointers must be free'd manually
in calling application. This is unfortunately not documented
well behaviour of popt and we were having memory leaks due to
it.
This commit is contained in:
Ondrej Kozina
2020-04-23 14:29:50 +02:00
committed by Milan Broz
parent 4a43a2773a
commit 92b24fd758
5 changed files with 92 additions and 24 deletions

View File

@@ -129,6 +129,34 @@ static const char *null_action_argv[] = {NULL, NULL};
void tools_cleanup(void) void tools_cleanup(void)
{ {
FREE_AND_NULL(opt_cipher);
FREE_AND_NULL(opt_keyslot_cipher);
FREE_AND_NULL(opt_hash);
FREE_AND_NULL(opt_json_file);
FREE_AND_NULL(opt_key_file);
FREE_AND_NULL(opt_keyfile_stdin);
FREE_AND_NULL(opt_master_key_file);
FREE_AND_NULL(opt_header_backup_file);
FREE_AND_NULL(opt_uuid);
FREE_AND_NULL(opt_header_device);
FREE_AND_NULL(opt_type);
FREE_AND_NULL(opt_pbkdf);
FREE_AND_NULL(opt_priority);
FREE_AND_NULL(opt_integrity);
FREE_AND_NULL(opt_key_description);
FREE_AND_NULL(opt_label);
FREE_AND_NULL(opt_subsystem);
FREE_AND_NULL(opt_active_name);
FREE_AND_NULL(opt_resilience_mode);
FREE_AND_NULL(opt_resilience_hash);
FREE_AND_NULL(opt_reduce_size_str);
FREE_AND_NULL(opt_hotzone_size_str);
FREE_AND_NULL(opt_device_size_str);
FREE_AND_NULL(opt_luks2_metadata_size_str);
FREE_AND_NULL(opt_luks2_keyslots_size_str);
while (opt_keyfiles_count)
free(opt_keyfiles[--opt_keyfiles_count]);
} }
static const char *uuid_or_device_header(const char **data_device) static const char *uuid_or_device_header(const char **data_device)
@@ -3460,10 +3488,12 @@ static void help(poptContext popt_context,
#if defined(ENABLE_LUKS_ADJUST_XTS_KEYSIZE) && DEFAULT_LUKS1_KEYBITS != 512 #if defined(ENABLE_LUKS_ADJUST_XTS_KEYSIZE) && DEFAULT_LUKS1_KEYBITS != 512
log_std(_("\tLUKS: Default keysize with XTS mode (two internal keys) will be doubled.\n")); log_std(_("\tLUKS: Default keysize with XTS mode (two internal keys) will be doubled.\n"));
#endif #endif
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
exit(EXIT_SUCCESS); exit(EXIT_SUCCESS);
} else if (key->shortName == 'V') { } else if (key->shortName == 'V') {
log_std("%s %s\n", PACKAGE_NAME, PACKAGE_VERSION); log_std("%s %s\n", PACKAGE_NAME, PACKAGE_VERSION);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
exit(EXIT_SUCCESS); exit(EXIT_SUCCESS);
} else } else
@@ -3504,7 +3534,6 @@ static int run_action(struct action_type *action)
int main(int argc, const char **argv) int main(int argc, const char **argv)
{ {
static char *popt_tmp;
static struct poptOption popt_help_options[] = { static struct poptOption popt_help_options[] = {
{ NULL, '\0', POPT_ARG_CALLBACK, help, 0, NULL, NULL }, { NULL, '\0', POPT_ARG_CALLBACK, help, 0, NULL, NULL },
{ "help", '?', POPT_ARG_NONE, NULL, 0, N_("Show this help message"), NULL }, { "help", '?', POPT_ARG_NONE, NULL, 0, N_("Show this help message"), NULL },
@@ -3520,19 +3549,19 @@ int main(int argc, const char **argv)
{ "cipher", 'c', POPT_ARG_STRING, &opt_cipher, 0, N_("The cipher used to encrypt the disk (see /proc/crypto)"), NULL }, { "cipher", 'c', POPT_ARG_STRING, &opt_cipher, 0, N_("The cipher used to encrypt the disk (see /proc/crypto)"), NULL },
{ "hash", 'h', POPT_ARG_STRING, &opt_hash, 0, N_("The hash used to create the encryption key from the passphrase"), NULL }, { "hash", 'h', POPT_ARG_STRING, &opt_hash, 0, N_("The hash used to create the encryption key from the passphrase"), NULL },
{ "verify-passphrase", 'y', POPT_ARG_NONE, &opt_verify_passphrase, 0, N_("Verifies the passphrase by asking for it twice"), NULL }, { "verify-passphrase", 'y', POPT_ARG_NONE, &opt_verify_passphrase, 0, N_("Verifies the passphrase by asking for it twice"), NULL },
{ "key-file", 'd', POPT_ARG_STRING, &opt_key_file, 6, N_("Read the key from a file"), NULL }, { "key-file", 'd', POPT_ARG_STRING, NULL, 6, N_("Read the key from a file"), NULL },
{ "master-key-file", '\0', POPT_ARG_STRING, &opt_master_key_file, 0, N_("Read the volume (master) key from file."), NULL }, { "master-key-file", '\0', POPT_ARG_STRING, &opt_master_key_file, 0, N_("Read the volume (master) key from file."), NULL },
{ "dump-master-key", '\0', POPT_ARG_NONE, &opt_dump_master_key, 0, N_("Dump volume (master) key instead of keyslots info"), NULL }, { "dump-master-key", '\0', POPT_ARG_NONE, &opt_dump_master_key, 0, N_("Dump volume (master) key instead of keyslots info"), NULL },
{ "key-size", 's', POPT_ARG_INT, &opt_key_size, 0, N_("The size of the encryption key"), N_("BITS") }, { "key-size", 's', POPT_ARG_INT, &opt_key_size, 0, N_("The size of the encryption key"), N_("BITS") },
{ "keyfile-size", 'l', POPT_ARG_LONG, &opt_keyfile_size, 0, N_("Limits the read from keyfile"), N_("bytes") }, { "keyfile-size", 'l', POPT_ARG_LONG, &opt_keyfile_size, 0, N_("Limits the read from keyfile"), N_("bytes") },
{ "keyfile-offset", '\0', POPT_ARG_STRING, &popt_tmp, 4, N_("Number of bytes to skip in keyfile"), N_("bytes") }, { "keyfile-offset", '\0', POPT_ARG_STRING, NULL, 4, N_("Number of bytes to skip in keyfile"), N_("bytes") },
{ "new-keyfile-size", '\0', POPT_ARG_LONG, &opt_new_keyfile_size, 0, N_("Limits the read from newly added keyfile"), N_("bytes") }, { "new-keyfile-size", '\0', POPT_ARG_LONG, &opt_new_keyfile_size, 0, N_("Limits the read from newly added keyfile"), N_("bytes") },
{ "new-keyfile-offset",'\0', POPT_ARG_STRING, &popt_tmp, 5, N_("Number of bytes to skip in newly added keyfile"), N_("bytes") }, { "new-keyfile-offset",'\0', POPT_ARG_STRING, NULL, 5, N_("Number of bytes to skip in newly added keyfile"), N_("bytes") },
{ "key-slot", 'S', POPT_ARG_INT, &opt_key_slot, 0, N_("Slot number for new key (default is first free)"), NULL }, { "key-slot", 'S', POPT_ARG_INT, &opt_key_slot, 0, N_("Slot number for new key (default is first free)"), NULL },
{ "size", 'b', POPT_ARG_STRING, &popt_tmp, 1, N_("The size of the device"), N_("SECTORS") }, { "size", 'b', POPT_ARG_STRING, NULL, 1, N_("The size of the device"), N_("SECTORS") },
{ "device-size", '\0', POPT_ARG_STRING, &opt_device_size_str, 0, N_("Use only specified device size (ignore rest of device). DANGEROUS!"), N_("bytes") }, { "device-size", '\0', POPT_ARG_STRING, &opt_device_size_str, 0, N_("Use only specified device size (ignore rest of device). DANGEROUS!"), N_("bytes") },
{ "offset", 'o', POPT_ARG_STRING, &popt_tmp, 2, N_("The start offset in the backend device"), N_("SECTORS") }, { "offset", 'o', POPT_ARG_STRING, NULL, 2, N_("The start offset in the backend device"), N_("SECTORS") },
{ "skip", 'p', POPT_ARG_STRING, &popt_tmp, 3, N_("How many sectors of the encrypted data to skip at the beginning"), N_("SECTORS") }, { "skip", 'p', POPT_ARG_STRING, NULL, 3, N_("How many sectors of the encrypted data to skip at the beginning"), N_("SECTORS") },
{ "readonly", 'r', POPT_ARG_NONE, &opt_readonly, 0, N_("Create a readonly mapping"), NULL }, { "readonly", 'r', POPT_ARG_NONE, &opt_readonly, 0, N_("Create a readonly mapping"), NULL },
{ "batch-mode", 'q', POPT_ARG_NONE, &opt_batch_mode, 0, N_("Do not ask for confirmation"), NULL }, { "batch-mode", 'q', POPT_ARG_NONE, &opt_batch_mode, 0, N_("Do not ask for confirmation"), NULL },
{ "timeout", 't', POPT_ARG_INT, &opt_timeout, 0, N_("Timeout for interactive passphrase prompt (in seconds)"), N_("secs") }, { "timeout", 't', POPT_ARG_INT, &opt_timeout, 0, N_("Timeout for interactive passphrase prompt (in seconds)"), N_("secs") },
@@ -3616,25 +3645,29 @@ int main(int argc, const char **argv)
while((r = poptGetNextOpt(popt_context)) > 0) { while((r = poptGetNextOpt(popt_context)) > 0) {
unsigned long long ull_value; unsigned long long ull_value;
char *endp; char *endp, *str = poptGetOptArg(popt_context);
if (r == 6) { if (r == 6) {
char *kf = poptGetOptArg(popt_context); free(opt_key_file);
if (tools_is_stdin(kf)) opt_key_file = str;
opt_keyfile_stdin = kf; if (tools_is_stdin(str)) {
else if (opt_keyfiles_count < MAX_KEYFILES) free(opt_keyfile_stdin);
opt_keyfiles[opt_keyfiles_count++] = kf; opt_keyfile_stdin = strdup(str);
} else if (opt_keyfiles_count < MAX_KEYFILES)
opt_keyfiles[opt_keyfiles_count++] = strdup(str);
total_keyfiles++; total_keyfiles++;
continue; continue;
} }
errno = 0; errno = 0;
ull_value = strtoull(popt_tmp, &endp, 0); ull_value = strtoull(str, &endp, 0);
if (*endp || !*popt_tmp || !isdigit(*popt_tmp) || if (*endp || !*str || !isdigit(*str) ||
(errno == ERANGE && ull_value == ULLONG_MAX) || (errno == ERANGE && ull_value == ULLONG_MAX) ||
(errno != 0 && ull_value == 0)) (errno != 0 && ull_value == 0))
r = POPT_ERROR_BADNUMBER; r = POPT_ERROR_BADNUMBER;
free(str);
switch(r) { switch(r) {
case 1: case 1:
opt_size = ull_value; opt_size = ull_value;
@@ -3991,6 +4024,7 @@ int main(int argc, const char **argv)
if (opt_disable_locks && crypt_metadata_locking(NULL, 0)) { if (opt_disable_locks && crypt_metadata_locking(NULL, 0)) {
log_std(_("Cannot disable metadata locking.")); log_std(_("Cannot disable metadata locking."));
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
exit(EXIT_FAILURE); exit(EXIT_FAILURE);
} }
@@ -4048,6 +4082,7 @@ int main(int argc, const char **argv)
poptGetInvocationName(popt_context)); poptGetInvocationName(popt_context));
r = run_action(action); r = run_action(action);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
return r; return r;
} }

View File

@@ -119,6 +119,8 @@ int tools_lookup_crypt_device(struct crypt_device *cd, const char *type,
/* each utility is required to implement it */ /* each utility is required to implement it */
void tools_cleanup(void); void tools_cleanup(void);
#define FREE_AND_NULL(x) do { free(x); x = NULL; } while (0)
/* Log */ /* Log */
#define log_dbg(x...) clogger(NULL, CRYPT_LOG_DEBUG, __FILE__, __LINE__, x) #define log_dbg(x...) clogger(NULL, CRYPT_LOG_DEBUG, __FILE__, __LINE__, x)
#define log_std(x...) clogger(NULL, CRYPT_LOG_NORMAL, __FILE__, __LINE__, x) #define log_std(x...) clogger(NULL, CRYPT_LOG_NORMAL, __FILE__, __LINE__, x)

View File

@@ -119,6 +119,16 @@ typedef enum {
void tools_cleanup(void) void tools_cleanup(void)
{ {
FREE_AND_NULL(opt_cipher);
FREE_AND_NULL(opt_hash);
FREE_AND_NULL(opt_key_file);
FREE_AND_NULL(opt_master_key_file);
FREE_AND_NULL(opt_uuid);
FREE_AND_NULL(opt_type);
FREE_AND_NULL(opt_pbkdf);
FREE_AND_NULL(opt_header_device);
FREE_AND_NULL(opt_reduce_size_str);
FREE_AND_NULL(opt_device_size_str);
} }
static void _quiet_log(int level, const char *msg, void *usrptr) static void _quiet_log(int level, const char *msg, void *usrptr)
@@ -1598,10 +1608,12 @@ static void help(poptContext popt_context,
if (key->shortName == '?') { if (key->shortName == '?') {
log_std("%s %s\n", PACKAGE_REENC, PACKAGE_VERSION); log_std("%s %s\n", PACKAGE_REENC, PACKAGE_VERSION);
poptPrintHelp(popt_context, stdout, 0); poptPrintHelp(popt_context, stdout, 0);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
exit(EXIT_SUCCESS); exit(EXIT_SUCCESS);
} else if (key->shortName == 'V') { } else if (key->shortName == 'V') {
log_std("%s %s\n", PACKAGE_REENC, PACKAGE_VERSION); log_std("%s %s\n", PACKAGE_REENC, PACKAGE_VERSION);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
exit(EXIT_SUCCESS); exit(EXIT_SUCCESS);
} else } else
@@ -1772,8 +1784,7 @@ int main(int argc, const char **argv)
} }
r = run_reencrypt(action_argv[0]); r = run_reencrypt(action_argv[0]);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
return translate_errno(r); return translate_errno(r);
} }

View File

@@ -65,6 +65,14 @@ static int action_argc;
void tools_cleanup(void) void tools_cleanup(void)
{ {
FREE_AND_NULL(opt_data_device);
FREE_AND_NULL(opt_integrity);
FREE_AND_NULL(opt_integrity_key_file);
FREE_AND_NULL(opt_journal_integrity);
FREE_AND_NULL(opt_journal_integrity_key_file);
FREE_AND_NULL(opt_journal_crypt);
FREE_AND_NULL(opt_journal_crypt_key_file);
FREE_AND_NULL(opt_journal_size_str);
} }
// FIXME: move this to tools and handle EINTR // FIXME: move this to tools and handle EINTR
@@ -504,10 +512,12 @@ static void help(poptContext popt_context,
log_std(_("\nDefault compiled-in dm-integrity parameters:\n" log_std(_("\nDefault compiled-in dm-integrity parameters:\n"
"\tChecksum algorithm: %s\n"), DEFAULT_ALG_NAME); "\tChecksum algorithm: %s\n"), DEFAULT_ALG_NAME);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
exit(EXIT_SUCCESS); exit(EXIT_SUCCESS);
} else if (key->shortName == 'V') { } else if (key->shortName == 'V') {
log_std("%s %s\n", PACKAGE_INTEGRITY, PACKAGE_VERSION); log_std("%s %s\n", PACKAGE_INTEGRITY, PACKAGE_VERSION);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
exit(EXIT_SUCCESS); exit(EXIT_SUCCESS);
} else } else
@@ -716,6 +726,7 @@ int main(int argc, const char **argv)
} }
r = run_action(action); r = run_action(action);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
return r; return r;
} }

View File

@@ -48,6 +48,11 @@ static int action_argc;
void tools_cleanup(void) void tools_cleanup(void)
{ {
FREE_AND_NULL(opt_fec_device);
FREE_AND_NULL(opt_hash_algorithm);
FREE_AND_NULL(opt_salt);
FREE_AND_NULL(opt_uuid);
FREE_AND_NULL(opt_root_hash_signature);
} }
static int _prepare_format(struct crypt_params_verity *params, static int _prepare_format(struct crypt_params_verity *params,
@@ -435,10 +440,12 @@ static void help(poptContext popt_context,
DEFAULT_VERITY_HASH, DEFAULT_VERITY_DATA_BLOCK, DEFAULT_VERITY_HASH, DEFAULT_VERITY_DATA_BLOCK,
DEFAULT_VERITY_HASH_BLOCK, DEFAULT_VERITY_SALT_SIZE, DEFAULT_VERITY_HASH_BLOCK, DEFAULT_VERITY_SALT_SIZE,
1); 1);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
exit(EXIT_SUCCESS); exit(EXIT_SUCCESS);
} else if (key->shortName == 'V') { } else if (key->shortName == 'V') {
log_std("%s %s\n", PACKAGE_VERITY, PACKAGE_VERSION); log_std("%s %s\n", PACKAGE_VERITY, PACKAGE_VERSION);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
exit(EXIT_SUCCESS); exit(EXIT_SUCCESS);
} else } else
@@ -459,7 +466,6 @@ static int run_action(struct action_type *action)
int main(int argc, const char **argv) int main(int argc, const char **argv)
{ {
static char *popt_tmp;
static const char *null_action_argv[] = {NULL}; static const char *null_action_argv[] = {NULL};
static struct poptOption popt_help_options[] = { static struct poptOption popt_help_options[] = {
{ NULL, '\0', POPT_ARG_CALLBACK, help, 0, NULL, NULL }, { NULL, '\0', POPT_ARG_CALLBACK, help, 0, NULL, NULL },
@@ -477,10 +483,10 @@ int main(int argc, const char **argv)
{ "data-block-size", 0, POPT_ARG_INT, &opt_data_block_size, 0, N_("Block size on the data device"), N_("bytes") }, { "data-block-size", 0, POPT_ARG_INT, &opt_data_block_size, 0, N_("Block size on the data device"), N_("bytes") },
{ "hash-block-size", 0, POPT_ARG_INT, &opt_hash_block_size, 0, N_("Block size on the hash device"), N_("bytes") }, { "hash-block-size", 0, POPT_ARG_INT, &opt_hash_block_size, 0, N_("Block size on the hash device"), N_("bytes") },
{ "fec-roots", 0, POPT_ARG_INT, &opt_fec_roots, 0, N_("FEC parity bytes"), N_("bytes") }, { "fec-roots", 0, POPT_ARG_INT, &opt_fec_roots, 0, N_("FEC parity bytes"), N_("bytes") },
{ "data-blocks", 0, POPT_ARG_STRING, &popt_tmp, 1, N_("The number of blocks in the data file"), N_("blocks") }, { "data-blocks", 0, POPT_ARG_STRING, NULL, 1, N_("The number of blocks in the data file"), N_("blocks") },
{ "fec-device", 0, POPT_ARG_STRING, &opt_fec_device, 0, N_("Path to device with error correction data"), N_("path") }, { "fec-device", 0, POPT_ARG_STRING, &opt_fec_device, 0, N_("Path to device with error correction data"), N_("path") },
{ "hash-offset", 0, POPT_ARG_STRING, &popt_tmp, 2, N_("Starting offset on the hash device"), N_("bytes") }, { "hash-offset", 0, POPT_ARG_STRING, NULL, 2, N_("Starting offset on the hash device"), N_("bytes") },
{ "fec-offset", 0, POPT_ARG_STRING, &popt_tmp, 3, N_("Starting offset on the FEC device"), N_("bytes") }, { "fec-offset", 0, POPT_ARG_STRING, NULL, 3, N_("Starting offset on the FEC device"), N_("bytes") },
{ "hash", 'h', POPT_ARG_STRING, &opt_hash_algorithm, 0, N_("Hash algorithm"), N_("string") }, { "hash", 'h', POPT_ARG_STRING, &opt_hash_algorithm, 0, N_("Hash algorithm"), N_("string") },
{ "salt", 's', POPT_ARG_STRING, &opt_salt, 0, N_("Salt"), N_("hex string") }, { "salt", 's', POPT_ARG_STRING, &opt_salt, 0, N_("Salt"), N_("hex string") },
{ "uuid", '\0', POPT_ARG_STRING, &opt_uuid, 0, N_("UUID for device to use"), NULL }, { "uuid", '\0', POPT_ARG_STRING, &opt_uuid, 0, N_("UUID for device to use"), NULL },
@@ -510,15 +516,17 @@ int main(int argc, const char **argv)
while((r = poptGetNextOpt(popt_context)) > 0) { while((r = poptGetNextOpt(popt_context)) > 0) {
unsigned long long ull_value; unsigned long long ull_value;
char *endp; char *endp, *str = poptGetOptArg(popt_context);
errno = 0; errno = 0;
ull_value = strtoull(popt_tmp, &endp, 10); ull_value = strtoull(str, &endp, 10);
if (*endp || !*popt_tmp || !isdigit(*popt_tmp) || if (*endp || !*str || !isdigit(*str) ||
(errno == ERANGE && ull_value == ULLONG_MAX) || (errno == ERANGE && ull_value == ULLONG_MAX) ||
(errno != 0 && ull_value == 0)) (errno != 0 && ull_value == 0))
r = POPT_ERROR_BADNUMBER; r = POPT_ERROR_BADNUMBER;
free(str);
switch(r) { switch(r) {
case 1: case 1:
data_blocks = ull_value; data_blocks = ull_value;
@@ -614,6 +622,7 @@ int main(int argc, const char **argv)
} }
r = run_action(action); r = run_action(action);
tools_cleanup();
poptFreeContext(popt_context); poptFreeContext(popt_context);
return r; return r;
} }