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);
}