On 10/01/17(Tue) 10:37, Martin Pieuchot wrote:
> In pfkey_sendup() we call sorwakeup() which asserts for NET_LOCK(), so
> we are already at IPL_SOFTNET.
>
> pfkeyv2_send() is called via pfkey_output() which is also called with
> the NET_LOCK() held.
>
> Finally replace a comment above pfkeyv2_ipo_walk() by the corresponding
> assert.
Anybody tested or reviewed this diff?
> Index: net/pfkey.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pfkey.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 pfkey.c
> --- net/pfkey.c 29 Nov 2016 10:22:30 -0000 1.33
> +++ net/pfkey.c 10 Jan 2017 09:29:59 -0000
> @@ -136,7 +136,8 @@ int
> pfkey_sendup(struct socket *socket, struct mbuf *packet, int more)
> {
> struct mbuf *packet2;
> - int s;
> +
> + splsoftassert(IPL_SOFTNET);
>
> if (more) {
> if (!(packet2 = m_dup_pkt(packet, 0, M_DONTWAIT)))
> @@ -144,13 +145,10 @@ pfkey_sendup(struct socket *socket, stru
> } else
> packet2 = packet;
>
> - s = splsoftnet();
> if (!sbappendaddr(&socket->so_rcv, &pfkey_addr, packet2, NULL)) {
> m_freem(packet2);
> - splx(s);
> return (ENOBUFS);
> }
> - splx(s);
>
> sorwakeup(socket);
> return (0);
> Index: net/pfkeyv2.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.148
> diff -u -p -r1.148 pfkeyv2.c
> --- net/pfkeyv2.c 15 Sep 2016 02:00:18 -0000 1.148
> +++ net/pfkeyv2.c 10 Jan 2017 09:29:41 -0000
> @@ -783,7 +783,7 @@ int
> pfkeyv2_send(struct socket *socket, void *message, int len)
> {
> int i, j, rval = 0, mode = PFKEYV2_SENDMESSAGE_BROADCAST;
> - int delflag = 0, s;
> + int delflag = 0;
> struct sockaddr_encap encapdst, encapnetmask;
> struct ipsec_policy *ipo, *tmpipo;
> struct ipsec_acquire *ipa;
> @@ -807,6 +807,8 @@ pfkeyv2_send(struct socket *socket, void
>
> u_int rdomain;
>
> + splsoftassert(IPL_SOFTNET);
> +
> /* Verify that we received this over a legitimate pfkeyv2 socket */
> bzero(headers, sizeof(headers));
>
> @@ -940,8 +942,6 @@ pfkeyv2_send(struct socket *socket, void
> }
> #endif /* IPSEC */
>
> - s = splsoftnet();
> -
> /* Find TDB */
> sa2 = gettdb(rdomain, ssa->sadb_sa_spi, sunionp,
> SADB_X_GETSPROTO(smsg->sadb_msg_satype));
> @@ -949,7 +949,7 @@ pfkeyv2_send(struct socket *socket, void
> /* If there's no such SA, we're done */
> if (sa2 == NULL) {
> rval = ESRCH;
> - goto splxret;
> + goto ret;
> }
>
> /* If this is a reserved SA */
> @@ -969,7 +969,7 @@ pfkeyv2_send(struct socket *socket, void
> &newsa->tdb_sproto, &alg))) {
> tdb_free(freeme);
> freeme = NULL;
> - goto splxret;
> + goto ret;
> }
>
> /* Initialize SA */
> @@ -1020,7 +1020,7 @@ pfkeyv2_send(struct socket *socket, void
> rval = EINVAL;
> tdb_free(freeme);
> freeme = NULL;
> - goto splxret;
> + goto ret;
> }
>
> newsa->tdb_cur_allocations = sa2->tdb_cur_allocations;
> @@ -1041,7 +1041,7 @@ pfkeyv2_send(struct socket *socket, void
> headers[SADB_EXT_IDENTITY_DST] ||
> headers[SADB_EXT_SENSITIVITY]) {
> rval = EINVAL;
> - goto splxret;
> + goto ret;
> }
>
> import_sa(sa2, headers[SADB_EXT_SA], NULL);
> @@ -1059,7 +1059,6 @@ pfkeyv2_send(struct socket *socket, void
> #endif
> }
>
> - splx(s);
> break;
> case SADB_ADD:
> ssa = (struct sadb_sa *) headers[SADB_EXT_SA];
> @@ -1092,21 +1091,19 @@ pfkeyv2_send(struct socket *socket, void
> }
> #endif /* IPSEC */
>
> - s = splsoftnet();
> -
> sa2 = gettdb(rdomain, ssa->sadb_sa_spi, sunionp,
> SADB_X_GETSPROTO(smsg->sadb_msg_satype));
>
> /* We can't add an existing SA! */
> if (sa2 != NULL) {
> rval = EEXIST;
> - goto splxret;
> + goto ret;
> }
>
> /* We can only add "mature" SAs */
> if (ssa->sadb_sa_state != SADB_SASTATE_MATURE) {
> rval = EINVAL;
> - goto splxret;
> + goto ret;
> }
>
> /* Allocate and initialize new TDB */
> @@ -1124,7 +1121,7 @@ pfkeyv2_send(struct socket *socket, void
> &newsa->tdb_sproto, &alg))) {
> tdb_free(freeme);
> freeme = NULL;
> - goto splxret;
> + goto ret;
> }
>
> import_sa(newsa, headers[SADB_EXT_SA], &ii);
> @@ -1177,15 +1174,13 @@ pfkeyv2_send(struct socket *socket, void
> rval = EINVAL;
> tdb_free(freeme);
> freeme = NULL;
> - goto splxret;
> + goto ret;
> }
> }
>
> /* Add TDB in table */
> puttdb((struct tdb *) freeme);
>
> - splx(s);
> -
> freeme = NULL;
> break;
>
> @@ -1194,19 +1189,16 @@ pfkeyv2_send(struct socket *socket, void
> sunionp =
> (union sockaddr_union *)(headers[SADB_EXT_ADDRESS_DST] +
> sizeof(struct sadb_address));
> - s = splsoftnet();
>
> sa2 = gettdb(rdomain, ssa->sadb_sa_spi, sunionp,
> SADB_X_GETSPROTO(smsg->sadb_msg_satype));
> if (sa2 == NULL) {
> rval = ESRCH;
> - goto splxret;
> + goto ret;
> }
>
> tdb_delete(sa2);
>
> - splx(s);
> -
> sa2 = NULL;
> break;
>
> @@ -1230,21 +1222,17 @@ pfkeyv2_send(struct socket *socket, void
> (union sockaddr_union *)(headers[SADB_EXT_ADDRESS_DST] +
> sizeof(struct sadb_address));
>
> - s = splsoftnet();
> -
> sa2 = gettdb(rdomain, ssa->sadb_sa_spi, sunionp,
> SADB_X_GETSPROTO(smsg->sadb_msg_satype));
> if (sa2 == NULL) {
> rval = ESRCH;
> - goto splxret;
> + goto ret;
> }
>
> rval = pfkeyv2_get(sa2, headers, &freeme, NULL);
> if (rval)
> mode = PFKEYV2_SENDMESSAGE_UNICAST;
>
> - splx(s);
> -
> break;
>
> case SADB_REGISTER:
> @@ -1323,15 +1311,12 @@ pfkeyv2_send(struct socket *socket, void
>
> switch (smsg->sadb_msg_satype) {
> case SADB_SATYPE_UNSPEC:
> - s = splsoftnet();
> -
> for (ipo = TAILQ_FIRST(&ipsec_policy_head);
> ipo != NULL; ipo = tmpipo) {
> tmpipo = TAILQ_NEXT(ipo, ipo_list);
> if (ipo->ipo_rdomain == rdomain)
> ipsec_delete_policy(ipo);
> }
> - splx(s);
> /* FALLTHROUGH */
> case SADB_SATYPE_AH:
> case SADB_SATYPE_ESP:
> @@ -1340,12 +1325,9 @@ pfkeyv2_send(struct socket *socket, void
> #ifdef TCP_SIGNATURE
> case SADB_X_SATYPE_TCPSIGNATURE:
> #endif /* TCP_SIGNATURE */
> - s = splsoftnet();
> -
> tdb_walk(rdomain, pfkeyv2_flush_walker,
> (u_int8_t *) &(smsg->sadb_msg_satype));
>
> - splx(s);
> break;
>
> default:
> @@ -1360,10 +1342,7 @@ pfkeyv2_send(struct socket *socket, void
> dump_state.sadb_msg = (struct sadb_msg *) headers[0];
> dump_state.socket = socket;
>
> - s = splsoftnet();
> rval = tdb_walk(rdomain, pfkeyv2_dump_walker, &dump_state);
> - splx(s);
> -
> if (!rval)
> goto realret;
>
> @@ -1381,13 +1360,11 @@ pfkeyv2_send(struct socket *socket, void
> sunionp = (union sockaddr_union *)
> (headers[SADB_EXT_ADDRESS_DST] +
> sizeof(struct sadb_address));
>
> - s = splsoftnet();
> -
> tdb1 = gettdb(rdomain, ssa->sadb_sa_spi, sunionp,
> SADB_X_GETSPROTO(smsg->sadb_msg_satype));
> if (tdb1 == NULL) {
> rval = ESRCH;
> - goto splxret;
> + goto ret;
> }
>
> ssa = (struct sadb_sa *) headers[SADB_X_EXT_SA2];
> @@ -1399,14 +1376,14 @@ pfkeyv2_send(struct socket *socket, void
> SADB_X_GETSPROTO(sa_proto->sadb_protocol_proto));
> if (tdb2 == NULL) {
> rval = ESRCH;
> - goto splxret;
> + goto ret;
> }
>
> /* Detect cycles */
> for (tdb3 = tdb2; tdb3; tdb3 = tdb3->tdb_onext)
> if (tdb3 == tdb1) {
> rval = ESRCH;
> - goto splxret;
> + goto ret;
> }
>
> /* Maintenance */
> @@ -1421,8 +1398,6 @@ pfkeyv2_send(struct socket *socket, void
> /* Link them */
> tdb1->tdb_onext = tdb2;
> tdb2->tdb_inext = tdb1;
> -
> - splx(s);
> }
> break;
>
> @@ -1472,7 +1447,6 @@ pfkeyv2_send(struct socket *socket, void
> headers[SADB_X_EXT_PROTOCOL],
> headers[SADB_X_EXT_FLOW_TYPE]);
>
> /* Determine whether the exact same SPD entry already exists. */
> - s = splsoftnet();
> if ((rn = rn_match(&encapdst, rnh)) != NULL) {
> ipo = (struct ipsec_policy *)rn;
>
> @@ -1494,7 +1468,6 @@ pfkeyv2_send(struct socket *socket, void
> if (exists && (ipo->ipo_flags & IPSP_POLICY_STATIC)) {
> if (!(sab->sadb_protocol_flags &
> SADB_X_POLICYFLAGS_POLICY)) {
> - splx(s);
> goto ret;
> }
> }
> @@ -1503,12 +1476,10 @@ pfkeyv2_send(struct socket *socket, void
> if (delflag) {
> if (exists) {
> rval = ipsec_delete_policy(ipo);
> - splx(s);
> goto ret;
> }
>
> /* If we were asked to delete something non-existent,
> error. */
> - splx(s);
> rval = ESRCH;
> break;
> }
> @@ -1524,7 +1495,6 @@ pfkeyv2_send(struct socket *socket, void
> /* Allocate policy entry */
> ipo = pool_get(&ipsec_policy_pool, PR_NOWAIT|PR_ZERO);
> if (ipo == NULL) {
> - splx(s);
> rval = ENOMEM;
> goto ret;
> }
> @@ -1561,7 +1531,6 @@ pfkeyv2_send(struct socket *socket, void
> else
> ipsec_delete_policy(ipo);
>
> - splx(s);
> rval = EINVAL;
> goto ret;
> }
> @@ -1596,7 +1565,6 @@ pfkeyv2_send(struct socket *socket, void
> ipsec_delete_policy(ipo);
> else
> pool_put(&ipsec_policy_pool, ipo);
> - splx(s);
> rval = ENOBUFS;
> goto ret;
> }
> @@ -1628,7 +1596,6 @@ pfkeyv2_send(struct socket *socket, void
> ipsp_ids_free(ipo->ipo_ids);
> pool_put(&ipsec_policy_pool, ipo);
>
> - splx(s);
> goto ret;
> }
> TAILQ_INSERT_HEAD(&ipsec_policy_head, ipo, ipo_list);
> @@ -1636,8 +1603,6 @@ pfkeyv2_send(struct socket *socket, void
> } else {
> ipo->ipo_last_searched = ipo->ipo_flags = 0;
> }
> -
> - splx(s);
> }
> break;
>
> @@ -1678,7 +1643,6 @@ pfkeyv2_send(struct socket *socket, void
> }
> }
> }
> -
>
> break;
>
> @@ -1725,10 +1689,6 @@ realret:
> free(sa1, M_PFKEY, 0);
>
> return (rval);
> -
> -splxret:
> - splx(s);
> - goto ret;
> }
>
> /*
> @@ -2187,15 +2147,14 @@ ret:
> return (rval);
> }
>
> -/*
> - * Caller is responsible for setting at least splsoftnet().
> - */
> int
> pfkeyv2_ipo_walk(u_int rdomain, int (*walker)(struct ipsec_policy *, void *),
> void *arg)
> {
> int rval = 0;
> struct ipsec_policy *ipo;
> +
> + splsoftassert(IPL_SOFTNET);
>
> TAILQ_FOREACH(ipo, &ipsec_policy_head, ipo_list) {
> if (ipo->ipo_rdomain != rdomain)
>
