Hi,
Using void* for temporary allocated TDB in pfkeyv2 does not make
sense. It looks like this is from a time before we had TDB pools.
Do not use the freeme pointer for TDB in pfkeyv2_send(). The pattern
is tdb_alloc() and tdb_unref() in case of error. Replace tdb_free()
in reserve_spi() with tdb_unref() to keep this pattern consistent.
Only tdb_unref() should call tdb_free().
No functional change.
ok?
bluhm
Index: net/pfkeyv2.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.222
diff -u -p -r1.222 pfkeyv2.c
--- net/pfkeyv2.c 25 Nov 2021 13:46:02 -0000 1.222
+++ net/pfkeyv2.c 25 Nov 2021 22:43:04 -0000
@@ -1327,22 +1327,18 @@ pfkeyv2_send(struct socket *so, void *me
int alg;
/* Create new TDB */
- freeme_sz = 0;
- freeme = tdb_alloc(rdomain);
- bzero(&ii, sizeof(struct ipsecinit));
-
- newsa = (struct tdb *) freeme;
+ newsa = tdb_alloc(rdomain);
newsa->tdb_satype = smsg->sadb_msg_satype;
if ((rval = pfkeyv2_get_proto_alg(newsa->tdb_satype,
&newsa->tdb_sproto, &alg))) {
- tdb_unref(freeme);
- freeme = NULL;
+ tdb_unref(newsa);
NET_UNLOCK();
goto ret;
}
/* Initialize SA */
+ bzero(&ii, sizeof(struct ipsecinit));
import_sa(newsa, headers[SADB_EXT_SA], &ii);
import_address(&newsa->tdb_src.sa,
headers[SADB_EXT_ADDRESS_SRC]);
@@ -1372,8 +1368,7 @@ pfkeyv2_send(struct socket *so, void *me
headers[SADB_X_EXT_DST_MASK],
headers[SADB_X_EXT_PROTOCOL],
headers[SADB_X_EXT_FLOW_TYPE]))) {
- tdb_unref(freeme);
- freeme = NULL;
+ tdb_unref(newsa);
NET_UNLOCK();
goto ret;
}
@@ -1395,8 +1390,7 @@ pfkeyv2_send(struct socket *so, void *me
rval = tdb_init(newsa, alg, &ii);
if (rval) {
rval = EINVAL;
- tdb_unref(freeme);
- freeme = NULL;
+ tdb_unref(newsa);
NET_UNLOCK();
goto ret;
}
@@ -1405,8 +1399,7 @@ pfkeyv2_send(struct socket *so, void *me
/* Delete old version of the SA, insert new one */
tdb_delete(sa2);
- puttdb((struct tdb *) freeme);
- freeme = NULL;
+ puttdb(newsa);
} else {
/*
* The SA is already initialized, so we're only allowed
to
@@ -1498,26 +1491,24 @@ pfkeyv2_send(struct socket *so, void *me
goto ret;
}
- /* Allocate and initialize new TDB */
- freeme_sz = 0;
- freeme = tdb_alloc(rdomain);
-
{
- struct tdb *newsa = (struct tdb *) freeme;
+ struct tdb *newsa;
struct ipsecinit ii;
int alg;
- bzero(&ii, sizeof(struct ipsecinit));
-
+ /* Create new TDB */
+ newsa = tdb_alloc(rdomain);
newsa->tdb_satype = smsg->sadb_msg_satype;
+
if ((rval = pfkeyv2_get_proto_alg(newsa->tdb_satype,
&newsa->tdb_sproto, &alg))) {
- tdb_unref(freeme);
- freeme = NULL;
+ tdb_unref(newsa);
NET_UNLOCK();
goto ret;
}
+ /* Initialize SA */
+ bzero(&ii, sizeof(struct ipsecinit));
import_sa(newsa, headers[SADB_EXT_SA], &ii);
import_address(&newsa->tdb_src.sa,
headers[SADB_EXT_ADDRESS_SRC]);
@@ -1550,8 +1541,7 @@ pfkeyv2_send(struct socket *so, void *me
headers[SADB_X_EXT_DST_MASK],
headers[SADB_X_EXT_PROTOCOL],
headers[SADB_X_EXT_FLOW_TYPE]))) {
- tdb_unref(freeme);
- freeme = NULL;
+ tdb_unref(newsa);
NET_UNLOCK();
goto ret;
}
@@ -1573,18 +1563,16 @@ pfkeyv2_send(struct socket *so, void *me
rval = tdb_init(newsa, alg, &ii);
if (rval) {
rval = EINVAL;
- tdb_unref(freeme);
- freeme = NULL;
+ tdb_unref(newsa);
NET_UNLOCK();
goto ret;
}
- }
- /* Add TDB in table */
- puttdb((struct tdb *) freeme);
+ /* Add TDB in table */
+ puttdb(newsa);
+ }
NET_UNLOCK();
- freeme = NULL;
break;
case SADB_DELETE:
Index: netinet/ip_ipsp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v
retrieving revision 1.254
diff -u -p -r1.254 ip_ipsp.c
--- netinet/ip_ipsp.c 25 Nov 2021 13:46:02 -0000 1.254
+++ netinet/ip_ipsp.c 25 Nov 2021 22:43:04 -0000
@@ -324,7 +324,7 @@ reserve_spi(u_int rdomain, u_int32_t ssp
}
(*errval) = EEXIST;
- tdb_free(tdbp);
+ tdb_unref(tdbp);
return 0;
}
