Hi, pfkeyv2.c:1091:pfkeyv2_send(struct socket *so, void *message, int len) leaks memory in the SADB_REGISTER case (line 1579). It reuses void *freeme multiple times to build up void *headers[].
headers[] are bcopy'ed to another buffer inside of pfkeyv2_sendmessage() (line 2064) so as the name implies *freeme is safe to be freed at the end. Found by llvm/scan-build. Also don't check for NULL before free(), could do a pass over the entire file. Only compile tested. - Ben Index: pfkeyv2.c =================================================================== RCS file: /cvs/src/sys/net/pfkeyv2.c,v retrieving revision 1.198 diff -u -p -r1.198 pfkeyv2.c --- pfkeyv2.c 17 Jul 2019 18:52:46 -0000 1.198 +++ pfkeyv2.c 3 Mar 2020 17:58:22 -0000 @@ -1098,7 +1098,8 @@ pfkeyv2_send(struct socket *so, void *me struct radix_node_head *rnh; struct radix_node *rn = NULL; struct pkpcb *kp, *bkp; - void *freeme = NULL, *bckptr = NULL; + void *freeme = NULL, *freeme2 = NULL, *freeme3 = NULL; + void *bckptr = NULL; void *headers[SADB_EXT_MAX + 1]; union sockaddr_union *sunionp; struct tdb *sa1 = NULL, *sa2 = NULL; @@ -1605,7 +1606,7 @@ pfkeyv2_send(struct socket *so, void *me i = sizeof(struct sadb_supported) + sizeof(aalgs); - if (!(freeme = malloc(i, M_PFKEY, M_NOWAIT | M_ZERO))) { + if (!(freeme2 = malloc(i, M_PFKEY, M_NOWAIT | M_ZERO))) { rval = ENOMEM; goto ret; } @@ -1616,34 +1617,34 @@ pfkeyv2_send(struct socket *so, void *me (1 << ((struct sadb_msg *)message)->sadb_msg_satype); keyunlock(kp, s); - ssup = (struct sadb_supported *) freeme; + ssup = (struct sadb_supported *) freeme2; ssup->sadb_supported_len = i / sizeof(uint64_t); { - void *p = freeme + sizeof(struct sadb_supported); + void *p = freeme2 + sizeof(struct sadb_supported); bcopy(&aalgs[0], p, sizeof(aalgs)); } - headers[SADB_EXT_SUPPORTED_AUTH] = freeme; + headers[SADB_EXT_SUPPORTED_AUTH] = freeme2; i = sizeof(struct sadb_supported) + sizeof(calgs); - if (!(freeme = malloc(i, M_PFKEY, M_NOWAIT | M_ZERO))) { + if (!(freeme3 = malloc(i, M_PFKEY, M_NOWAIT | M_ZERO))) { rval = ENOMEM; goto ret; } - ssup = (struct sadb_supported *) freeme; + ssup = (struct sadb_supported *) freeme3; ssup->sadb_supported_len = i / sizeof(uint64_t); { - void *p = freeme + sizeof(struct sadb_supported); + void *p = freeme3 + sizeof(struct sadb_supported); bcopy(&calgs[0], p, sizeof(calgs)); } - headers[SADB_X_EXT_SUPPORTED_COMP] = freeme; + headers[SADB_X_EXT_SUPPORTED_COMP] = freeme3; break; @@ -2064,14 +2065,14 @@ ret: realret: - if (freeme) - free(freeme, M_PFKEY, 0); + free(freeme, M_PFKEY, 0); + free(freeme2, M_PFKEY, 0); + free(freeme3, M_PFKEY, 0); explicit_bzero(message, len); free(message, M_PFKEY, 0); - if (sa1) - free(sa1, M_PFKEY, 0); + free(sa1, M_PFKEY, 0); return (rval); }