On 11/7/19 3:46 PM, Eric Biggers wrote:
> On Thu, Nov 07, 2019 at 10:50:59AM -0600, Eric Sandeen wrote:
>> Invalid arguments to add_enckey will leak the "arg" allocation,
>> so fix that.
>>
>> Fixes: ba71de04 ("xfs_io/encrypt: add 'add_enckey' command")
>> Fixes-coverity-id: 1454644
>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>> ---
>>
>> diff --git a/io/encrypt.c b/io/encrypt.c
>> index 17d61cfb..c6a4e190 100644
>> --- a/io/encrypt.c
>> +++ b/io/encrypt.c
>> @@ -696,6 +696,7 @@ add_enckey_f(int argc, char **argv)
>> goto out;
>> break;
>> default:
>> + free(arg);
>> return command_usage(&add_enckey_cmd);
>> }
>> }
>>
>
> The same leak happens later in the function too. How about this instead:
whoops yes it does. I kind of hate "retval = command_usage" but seeing the
memset of the key on the way out it's probably prudent to have one common
exit point after the function gets started.
Care to send this as a formal patch?
Thanks,
-Eric
> diff --git a/io/encrypt.c b/io/encrypt.c
> index 17d61cfb..de48c50c 100644
> --- a/io/encrypt.c
> +++ b/io/encrypt.c
> @@ -678,6 +678,7 @@ add_enckey_f(int argc, char **argv)
> int c;
> struct fscrypt_add_key_arg *arg;
> ssize_t raw_size;
> + int retval = 0;
>
> arg = calloc(1, sizeof(*arg) + FSCRYPT_MAX_KEY_SIZE + 1);
> if (!arg) {
> @@ -696,14 +697,17 @@ add_enckey_f(int argc, char **argv)
> goto out;
> break;
> default:
> - return command_usage(&add_enckey_cmd);
> + retval = command_usage(&add_enckey_cmd);
> + goto out;
> }
> }
> argc -= optind;
> argv += optind;
>
> - if (argc != 0)
> - return command_usage(&add_enckey_cmd);
> + if (argc != 0) {
> + retval = command_usage(&add_enckey_cmd);
> + goto out;
> + }
>
> raw_size = read_until_limit_or_eof(STDIN_FILENO, arg->raw,
> FSCRYPT_MAX_KEY_SIZE + 1);
> @@ -732,7 +736,7 @@ add_enckey_f(int argc, char **argv)
> out:
> memset(arg->raw, 0, FSCRYPT_MAX_KEY_SIZE + 1);
> free(arg);
> - return 0;
> + return retval;
> }
>
> static int
>
