[openssh] Fixes ASAN memory leak errors
Fixes the two memory leak errors that ASAN found in openssh-portable:
- one, a global variable is never freed. Handled by registering an
atexit().
- two, a local array took ownership of a pointer to a string that is
then never freed. Handled by registering a cleanup attribute, because
there are multiple return paths from the function, and this change is
made locally so we know our compiler supports the cleanup approach.
Hopefully this makes ASAN happier.
The fix is made in our fork of openssh-portable because:
- The local variable fix is compiler-dependent, so may not be applicable
in general to the upstream.
- Upstream seems to not care about issues like these, otherwise they'd
have long handled it.
Bug: 56434
Change-Id: I3139279fe9c8fde2e146561b47b632af52916bdb
Reviewed-on: https://fuchsia-review.googlesource.com/c/third_party/openssh-portable/+/462755
Reviewed-by: George Kulakowski <kulakowski@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
Fuchsia-Auto-Submit: Filip Filmar <fmil@google.com>
diff --git a/sshd.c b/sshd.c
index 6decac3..fe6d34f 100644
--- a/sshd.c
+++ b/sshd.c
@@ -255,6 +255,11 @@
/* global key/cert auth options. XXX move to permanent ssh->authctxt? */
struct sshauthopt *auth_opts = NULL;
+static void atexit_cleanup_auth_opts() {
+ if (auth_opts == NULL) { return; }
+ free(auth_opts);
+ auth_opts = NULL;
+}
/* sshd_config buffer */
struct sshbuf *cfg;
@@ -2190,6 +2195,7 @@
/* Set default key authentication options */
if ((auth_opts = sshauthopt_new_with_keys_defaults()) == NULL)
fatal("allocation failed");
+ atexit(atexit_cleanup_auth_opts);
/* prepare buffer to collect messages to display to user after login */
if ((loginmsg = sshbuf_new()) == NULL)
@@ -2330,6 +2336,14 @@
return 0;
}
+// Frees the memory pointed to by the char* pointed to by s.
+// Used for auto-freeing strings.
+static void charptr_auto_free(char **s) {
+ if (*s != NULL) {
+ free(*s);
+ }
+}
+
/* SSH2 key exchange */
static void
do_ssh2_kex(struct ssh *ssh)
@@ -2356,8 +2370,11 @@
ssh_packet_set_rekey_limits(ssh, options.rekey_limit,
options.rekey_interval);
- myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = compat_pkalg_proposal(
- list_hostkey_types());
+ // pkgalg_proposal owns this string.
+ __attribute__((cleanup(charptr_auto_free))) char *pkgalg_proposal =
+ compat_pkalg_proposal(list_hostkey_types());
+ // Ownership of pkgalg_proposal is not transferred.
+ myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = pkgalg_proposal;
/* start key exchange */
if ((r = kex_setup(ssh, myproposal)) != 0)
