svn commit: r309394 - head/sys/netpfil/pf
Gleb Smirnoff
glebius at FreeBSD.org
Wed Dec 7 21:08:27 UTC 2016
Marcel,
thanks for the fixes. While the problem with the first chunk
in pfsync_sendout() is obvious, the problem you are fixing in th
second chunk in the pfsync_delete_state() is not clear to me.
Can you please explain what scenario are you fixing there?
On Fri, Dec 02, 2016 at 06:15:59AM +0000, Marcel Moolenaar wrote:
M> Author: marcel
M> Date: Fri Dec 2 06:15:59 2016
M> New Revision: 309394
M> URL: https://svnweb.freebsd.org/changeset/base/309394
M>
M> Log:
M> Fix use-after-free bugs in pfsync(4)
M>
M> Use after free happens for state that is deleted. The reference
M> count is what prevents the state from being freed. When the
M> state is dequeued, the reference count is dropped and the memory
M> freed. We can't dereference the next pointer or re-queue the
M> state.
M>
M> MFC after: 1 week
M> Differential Revision: https://reviews.freebsd.org/D8671
M>
M> Modified:
M> head/sys/netpfil/pf/if_pfsync.c
M>
M> Modified: head/sys/netpfil/pf/if_pfsync.c
M> ==============================================================================
M> --- head/sys/netpfil/pf/if_pfsync.c Fri Dec 2 06:07:27 2016 (r309393)
M> +++ head/sys/netpfil/pf/if_pfsync.c Fri Dec 2 06:15:59 2016 (r309394)
M> @@ -1509,7 +1509,7 @@ pfsync_sendout(int schedswi)
M> struct ip *ip;
M> struct pfsync_header *ph;
M> struct pfsync_subheader *subh;
M> - struct pf_state *st;
M> + struct pf_state *st, *st_next;
M> struct pfsync_upd_req_item *ur;
M> int offset;
M> int q, count = 0;
M> @@ -1559,7 +1559,7 @@ pfsync_sendout(int schedswi)
M> offset += sizeof(*subh);
M>
M> count = 0;
M> - TAILQ_FOREACH(st, &sc->sc_qs[q], sync_list) {
M> + TAILQ_FOREACH_SAFE(st, &sc->sc_qs[q], sync_list, st_next) {
M> KASSERT(st->sync_state == q,
M> ("%s: st->sync_state == q",
M> __func__));
M> @@ -1931,6 +1931,8 @@ pfsync_delete_state(struct pf_state *st)
M> if (sc->sc_len == PFSYNC_MINPKT)
M> callout_reset(&sc->sc_tmo, 1 * hz, pfsync_timeout, V_pfsyncif);
M>
M> + pf_ref_state(st);
M> +
M> switch (st->sync_state) {
M> case PFSYNC_S_INS:
M> /* We never got to tell the world so just forget about it. */
M> @@ -1950,6 +1952,9 @@ pfsync_delete_state(struct pf_state *st)
M> default:
M> panic("%s: unexpected sync state %d", __func__, st->sync_state);
M> }
M> +
M> + pf_release_state(st);
M> +
M> PFSYNC_UNLOCK(sc);
M> }
M>
M> _______________________________________________
M> svn-src-all at freebsd.org mailing list
M> https://lists.freebsd.org/mailman/listinfo/svn-src-all
M> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"
--
Totus tuus, Glebius.
More information about the svn-src-head
mailing list