Hello,

Patch fixes potential use-after-free in pf_table.c:pfr_set_addrs():

 463         for (i = 0; i < size; i++) {
 ...
 483                         q = pfr_lookup_addr(tmpkt, &ad, 1);
 484                         if (q != NULL) {
 485                                 ad.pfra_fback = PFR_FB_DUPLICATE;
 486                                 goto _skip;
 487                         }
 488                         p = pfr_create_kentry(&ad);
 489                         if (p == NULL)
 490                                 senderr(ENOMEM);
 491                         if (pfr_route_kentry(tmpkt, p)) {
 492                                 pfr_destroy_kentry(p);
 493                                 ad.pfra_fback = PFR_FB_NONE;
 494                         } else {
 495                                 SLIST_INSERT_HEAD(&addq, p, pfrke_workq);
 496                                 ad.pfra_fback = PFR_FB_ADDED;
 497                                 xadd++;
 498                         }
 499                         if (p->pfrke_type == PFRKE_COST)
 500                                 kt->pfrkt_refcntcost++;
 501                         pfr_ktable_winfo_update(kt, p);
 502                 }
 503 _skip:
 504                 if (flags & PFR_FLAG_FEEDBACK)
 505                         if (COPYOUT(&ad, addr+i, sizeof(ad), flags))
 506                                 senderr(EFAULT);
 507         }


the things start to go downhill if PF takes branch at 492, where `p` gets
freed. The code jumps to 499 & 501, where PF steps to dead pointer. I think the
right thing is to use goto _skip; in that branch to avoid 499 et. al.
completely.

path is below.

OK?

thanks and
regards
sasha

--------8<---------------8<---------------8<------------------8<--------

Index: pf_table.c
===================================================================
RCS file: /cvs/src/sys/net/pf_table.c,v
retrieving revision 1.115
diff -u -p -r1.115 pf_table.c
--- pf_table.c  7 Oct 2015 11:57:44 -0000       1.115
+++ pf_table.c  3 Nov 2015 20:15:26 -0000
@@ -491,6 +491,7 @@ pfr_set_addrs(struct pfr_table *tbl, str
                        if (pfr_route_kentry(tmpkt, p)) {
                                pfr_destroy_kentry(p);
                                ad.pfra_fback = PFR_FB_NONE;
+                               goto _skip;
                        } else {
                                SLIST_INSERT_HEAD(&addq, p, pfrke_workq);
                                ad.pfra_fback = PFR_FB_ADDED;

Reply via email to