pf: rework pf_icmp_state_lookup() failure mode

If pf_icmp_state_lookup() finds a state but rejects it for not matching the
expected direction we should unlock the state (and NULL out *state). This
simplifies life for callers, and also ensures there's no confusion about what a
non-NULL returned state means.

Previously it could have been left in there by the caller, resulting in callers
unlocking the same state twice.

MFC after:	1 week
Sponsored by:	Rubicon Communications, LLC ("Netgate")

(cherry picked from commit 0578fe4922)
This commit is contained in:
Kristof Provost 2024-08-30 13:36:39 +02:00
parent b822e3fab4
commit 38f74de718
2 changed files with 15 additions and 9 deletions

View file

@ -359,8 +359,8 @@ struct pfi_dynaddr {
mtx_unlock(_s->lock); \
} while (0)
#else
#define PF_STATE_LOCK(s) mtx_lock(s->lock)
#define PF_STATE_UNLOCK(s) mtx_unlock(s->lock)
#define PF_STATE_LOCK(s) mtx_lock((s)->lock)
#define PF_STATE_UNLOCK(s) mtx_unlock((s)->lock)
#endif
#ifdef INVARIANTS

View file

@ -6704,6 +6704,8 @@ pf_icmp_state_lookup(struct pf_state_key_cmp *key, struct pf_pdesc *pd,
pf_print_state(*state);
printf("\n");
}
PF_STATE_UNLOCK(*state);
*state = NULL;
return (PF_DROP);
}
return (-1);
@ -6756,15 +6758,16 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif,
kif, virtual_id, virtual_type, icmp_dir, &iidx,
PF_ICMP_MULTI_NONE, 0);
if (ret >= 0) {
MPASS(*state == NULL);
if (ret == PF_DROP && pd->af == AF_INET6 &&
icmp_dir == PF_OUT) {
if (*state != NULL)
PF_STATE_UNLOCK((*state));
ret = pf_icmp_state_lookup(&key, pd, state, m, off,
pd->dir, kif, virtual_id, virtual_type,
icmp_dir, &iidx, multi, 0);
if (ret >= 0)
if (ret >= 0) {
MPASS(*state == NULL);
return (ret);
}
} else
return (ret);
}
@ -7171,8 +7174,10 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif,
ret = pf_icmp_state_lookup(&key, &pd2, state, m, off,
pd2.dir, kif, virtual_id, virtual_type,
icmp_dir, &iidx, PF_ICMP_MULTI_NONE, 1);
if (ret >= 0)
if (ret >= 0) {
MPASS(*state == NULL);
return (ret);
}
/* translate source/destination address, if necessary */
if ((*state)->key[PF_SK_WIRE] !=
@ -7227,16 +7232,17 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif,
pd->dir, kif, virtual_id, virtual_type,
icmp_dir, &iidx, PF_ICMP_MULTI_NONE, 1);
if (ret >= 0) {
MPASS(*state == NULL);
if (ret == PF_DROP && pd2.af == AF_INET6 &&
icmp_dir == PF_OUT) {
if (*state != NULL)
PF_STATE_UNLOCK((*state));
ret = pf_icmp_state_lookup(&key, &pd2,
state, m, off, pd->dir, kif,
virtual_id, virtual_type,
icmp_dir, &iidx, multi, 1);
if (ret >= 0)
if (ret >= 0) {
MPASS(*state == NULL);
return (ret);
}
} else
return (ret);
}