On Wed, Apr 11, 2012 at 12:20:30PM +0200, Henning Brauer wrote: > * Henning Brauer <lists-open...@bsws.de> [2012-04-11 11:26]: > > * Siju George <sgeorge....@gmail.com> [2012-04-10 08:16]: > > > On Tue, Apr 10, 2012 at 11:40 AM, Andres Perera <andre...@zoho.com> wrote: > > > > altering the max might have consequences i don't know about: > > > I will stick with 15 :-) > > > > actually, bumping it should be absolutely safe. > > > > pretty dumb limit actually, we should just dynamically allocate the > > pflogifs array. > > please try this & report back > > Index: if_pflog.c > =================================================================== > RCS file: /cvs/src/sys/net/if_pflog.c,v > retrieving revision 1.49 > diff -u -p -r1.49 if_pflog.c > --- if_pflog.c 3 Feb 2012 01:57:50 -0000 1.49 > +++ if_pflog.c 11 Apr 2012 10:19:56 -0000 > @@ -80,6 +80,7 @@ > #endif > > void pflogattach(int); > +int pflogifs_resize(size_t); > int pflogoutput(struct ifnet *, struct mbuf *, struct sockaddr *, > struct rtentry *); > int pflogioctl(struct ifnet *, u_long, caddr_t); > @@ -91,16 +92,14 @@ LIST_HEAD(, pflog_softc) pflogif_list; > struct if_clone pflog_cloner = > IF_CLONE_INITIALIZER("pflog", pflog_clone_create, pflog_clone_destroy); > > -struct ifnet *pflogifs[PFLOGIFS_MAX]; /* for fast access */ > -struct mbuf *pflog_mhdr = NULL, *pflog_mptr = NULL; > +int npflogifs = 0; > +struct ifnet **pflogifs = NULL; /* for fast access */ > +struct mbuf *pflog_mhdr = NULL, *pflog_mptr = NULL; > > void > pflogattach(int npflog) > { > - int i; > LIST_INIT(&pflogif_list); > - for (i = 0; i < PFLOGIFS_MAX; i++) > - pflogifs[i] = NULL; > if (pflog_mhdr == NULL) > if ((pflog_mhdr = m_get(M_DONTWAIT, MT_HEADER)) == NULL) > panic("pflogattach: no mbuf"); > @@ -111,14 +110,41 @@ pflogattach(int npflog) > } > > int > +pflogifs_resize(size_t n) > +{ > + struct ifnet **p; > + int i; > + > + if (n > SIZE_MAX / sizeof(struct ifnet)) > + return (EINVAL); > + if (n == 0) > + p = NULL; > + else > + if ((p = malloc(n * sizeof(struct ifnet), M_DEVBUF, > + M_NOWAIT|M_ZERO)) == NULL) > + return (ENOMEM);
don't you need two different index vars for this next section? > + for (i = 0; i < n; i++) > + if (i < npflogifs) > + p[i] = pflogifs[i]; > + else > + p[i] = NULL; something like the following with caveats that a) it is 5am-ish for me and b) i did not try compiling it: for (i = 0, j = 0; i < n; i++, j++) { for (; j < npflogifs && NULL == pflogifs[j]; j++) ; if (j == npflogifs) break; p[i] = pflogifs[j]; } for (; i < n; i++) p[i] = NULL; > + > + if(pflogifs) ^^ nit > + free(pflogifs, M_DEVBUF); > + pflogifs = p; > + npflogifs = n; > + return (0); > +} > + > +int > pflog_clone_create(struct if_clone *ifc, int unit) > { > struct ifnet *ifp; > struct pflog_softc *pflogif; > int s; > > - if (unit >= PFLOGIFS_MAX) > - return (EINVAL); > + if (unit + 1 > npflogifs && pflogifs_resize(unit + 1) != 0) > + return (ENOMEM); > > if ((pflogif = malloc(sizeof(*pflogif), > M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL) > @@ -154,11 +180,16 @@ int > pflog_clone_destroy(struct ifnet *ifp) > { > struct pflog_softc *pflogif = ifp->if_softc; > - int s; > + int s, i; > > s = splnet(); > pflogifs[pflogif->sc_unit] = NULL; > LIST_REMOVE(pflogif, sc_list); > + > + for (i = npflogifs; i > 0 && pflogifs[i - 1] != NULL; i--) > + ; /* nothing */ > + if (i < npflogifs) > + pflogifs_resize(i); /* error harmless here */ So, if the last pflogifs entry is NULL don't resize down? Not really questioning the logic, but want to make sure I understand that's what is meant, cause there is an easier check for that than the for()-loop. Caveats: a) 5am-ish, b) not familiar with code. --patrick > splx(s); > > if_detach(ifp); > @@ -225,7 +256,8 @@ pflog_packet(struct pf_pdesc *pd, u_int8 > if (rm == NULL || pd == NULL || pd->kif == NULL || pd->m == NULL) > return (-1); > > - if ((ifn = pflogifs[rm->logif]) == NULL || !ifn->if_bpf) > + if (rm->logif >= npflogifs || (ifn = pflogifs[rm->logif]) == NULL || > + !ifn->if_bpf) > return (0); > > bzero(&hdr, sizeof(hdr)); > Index: pf_ioctl.c > =================================================================== > RCS file: /cvs/src/sys/net/pf_ioctl.c,v > retrieving revision 1.250 > diff -u -p -r1.250 pf_ioctl.c > --- pf_ioctl.c 3 Apr 2012 15:09:03 -0000 1.250 > +++ pf_ioctl.c 11 Apr 2012 10:19:57 -0000 > @@ -2595,8 +2595,6 @@ pf_rule_copyin(struct pf_rule *from, str > #if NPFLOG > 0 > if (!to->log) > to->logif = 0; > - if (to->logif >= PFLOGIFS_MAX) > - return (EINVAL); > #endif > to->quick = from->quick; > to->ifnot = from->ifnot; > > -- > Henning Brauer, h...@bsws.de, henn...@openbsd.org > BS Web Services, http://bsws.de, Full-Service ISP > Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully > Managed > Henning Brauer Consulting, http://henningbrauer.com/