[20543] in Kerberos_V5_Development
Re: change_password resource cleanup?
daemon@ATHENA.MIT.EDU (Greg Hudson)
Sat Jun 28 02:22:45 2025
Message-ID: <0eb8158b-5095-4582-8721-92577ca264be@mit.edu>
Date: Sat, 28 Jun 2025 02:22:31 -0400
MIME-Version: 1.0
To: Joonas Tuomisto <jootuom@gmail.com>, krbdev@mit.edu
Content-Language: en-US
From: "Greg Hudson" <ghudson@mit.edu>
In-Reply-To: <CALuYaDz3-DjXTqsN+yz+PDKoALq0nzETcxmnLBEw6cN4L5zgCQ@mail.gmail.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: krbdev-bounces@mit.edu
On 6/28/25 00:50, Joonas Tuomisto wrote:
> krb5_change_password() returns the result_code_string and result_string
> krb5_data structures. Are you supposed to free() them - or I suppose more
> specifically, krb5_free_data() them after use?
They should be freed with krb5_free_data_contents() when the caller is
done with them.
krb5_free_data() would free the krb5_data pointer itself, which would be
asymmetric (the pointer is supplied by the caller and not allocated by
krb5_change_password()) and often a memory violation.
> The documentation for krb5_chpw_message() explicitly calls out that you
> should krb5_free_string() its message_out. For krb5_change_password() it
> does not.
We usually document freeing requirements for libkrb5 function outputs,
and it is indeed missing for krb5_change_password(). I have made a note
to fix this. (Also missing: result_code_string may be null, although
result_string is required.)
> Are these krb5_free_* functions always safe to call or should you only
> assume the structs exist (and require cleanup) when e.g.
> krb5_change_password() returns with result_code set?
There are early exit paths from krb5_change_password() that do not set
*result_code_string or *result_string. This is not really ideal; we
generally try to initialize output arguments in library APIs, but a lot
of libkrb5 was written before that became a common practice. I have
made a note to fix this.
(libgssapi_krb5 is much, much better about initializing output arguments
along all exit paths.)
krb5_free_*() functions are okay to call with null pointers, or with
structures containing null pointers (e.g. krb5_free_data_contents() on a
krb5_data structure whose data element is null).
> My code now has if (retval) ... if (result_code) ... sprinkled and I can
> already see myself how you could easily end up with resource leaks with C
> and different code paths. A common cleanup path helps.
In spite of krb5_change_password() not always setting output arguments,
the following should be safe:
krb5_error_code ret;
krb5_data result_code_string = { 0 }, result_string = { 0 };
...
ret = krb5_change_password(context, ..., &result_code_string,
&result_string);
if (ret)
goto cleanup;
...
cleanup:
krb5_free_data_contents(context, &result_code_string);
krb5_free_data_contents(context, &result_string);
_______________________________________________
krbdev mailing list krbdev@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev
